chore(docs): Update ruby install instructions in environment setup readme

[MajorLift]

Description

The current ruby install instructions in the "Environment Setup" readme are vague, and may cause gem to be run with the system root installation.

https://consensys.slack.com/archives/C02U025CVU4/p1728496388117329

This commit updates the instructions with commands that fix this issue.

Related issues

Manual testing steps

Screenshots/Recordings

Pre-merge author checklist

• I’ve followed MetaMask Contributor Docs and MetaMask Mobile Coding Standards.
• I've completed the PR template to the best of my ability
• I’ve included tests if applicable
• I’ve documented my code using JSDoc format if applicable
• I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

• I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
• I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

[github-actions]

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

[joaoloureirop]

It is expected that a MetaMask contributor picks a version manager of their choice, and follows the version manager documentation to install and configure it on their system.

rbenv is mentioned as an example of a ruby version manager

We should keep from being prescriptive on how to configure tools that are not maintained by us.

[MajorLift]

@joaoloureirop Would it help to add language emphasizing that the rbenv commands are examples? Or perhaps we could add more links to sections within the rbenv readme?

The primary target of this readme section probably isn't developers who have already set up ruby version managers themselves and don't need the instructions. Shouldn't it be a priority to unblock non-users of ruby by using the most obvious and unambiguous language possible?

[legobeat]

FWIW there is also the option of the repository Dockerfile:

# build image with bundled node/ruby/etc
$ podman build --build-arg UID=$(id -u) --build-arg GID=$(id -g) -t localhost/mm-mobile-builder -f scripts/docker/Dockerfile .

# run shell in container
$ podman run --rm -it --user $(id -u):$(id -g) --userns=keep-id:uid=$(id -u),gid=$(id -g) -v "$(pwd)":/app -w /app -v ~/.yarn:/home/node/.yarn -v ~/.cache/yarn:/home/node/.cache/yarn mm-mobile-build:debian bash

# run ruby directly
$ podman run --rm -it --user $(id -u):$(id -g) --userns=keep-id:uid=$(id -u),gid=$(id -g) -v "$(pwd)":/app -w /app mm-mobile-build:debian ruby --version

(taken to the next level: https://github.com/legobeat/l7-devenv)

Shouldn't it be a priority to unblock non-users of ruby by using the most obvious and unambiguous language possible?

I think it should also be a priority to steer developers towards secure solution and setups they can control. (No shade on rbenv or using it the proposed way)

I do think there are risks with the approach, though, specifically considering the undeterministic mess that CocoaPods have become.. By steering towards a specific blessed setup, we kind of take responsibility for that being Good Enough.

So kind of agree with @joaoloureirop

Would it help to add language emphasizing that the rbenv commands are examples? Or perhaps we could add more links to sections within the rbenv readme?

Both sound like nice improvements to me!

[joaoloureirop]

@joaoloureirop Would it help to add language emphasizing that the rbenv commands are examples? Or perhaps we could add more links to sections within the rbenv readme?

We could expand this section to offer better guidance to new contributors unfamiliar with the tool.

The primary target of this readme section probably isn't developers who have already set up ruby version managers themselves and don't need the instructions. Shouldn't it be a priority to unblock non-users of ruby by using the most obvious and unambiguous language possible?

The concern is with new contributors getting familiar with this tool since it will require intervention every time our .ruby-version changes.

We want to help new contributors as much as possible, and in this case, the best help is redirecting them to the official documentation.

For example, if a new contributor decides to use the version manager mentioned in our docs they can follow the hyperlink provided and go through the installation guide, which is only two steps when using a package manager (brew install rbenv && rbenv init)

The most important part that is often overlooked is to set up the shell to load the version manager.

Down the road when intervention is needed (installing a new ruby version, rehashing, removing...), the official documentation is the best help they can get.

[MajorLift]

@joaoloureirop @legobeat Thanks for the feedback! I understand the need to avoid endorsing a specific version manager, but feel like the reservations around providing instructions...

it will require intervention every time our .ruby-version changes.
Down the road when intervention is needed (installing a new ruby version, rehashing, removing...), the official documentation is the best help they can get.

... might apply to any external package we don't manage but provide instructions for in our readmes.

I attempted a rewrite but I'm not sure how well it addresses your concerns. If either of you have suggestions around adding stronger disclaimer language or editing/removing the additions I'd be happy to apply them.

[joaoloureirop]

... might apply to any external package we don't manage but provide instructions for in our readmes.

Instructions should be provided in our readmes when the configuration differs from the default or when the official docs miss the installation/configuration guide.
Other than that, official docs should be the single source of truth.

the node version manager section is as vague as the ruby version manager section for the same reason.

[joaoloureirop]

What is the added value here that the official docs don't already provide?

[MajorLift]

Providing immediate, explicit, unambiguous instructions would hopefully serve as a better first line of defense against troubleshooting threads like these, some of them quite huge:

• https://consensys.slack.com/archives/C02U025CVU4/p1729086334814599?thread_ts=1728571415.206109&cid=C02U025CVU4
• https://consensys.slack.com/archives/C02U025CVU4/p1724357838719499?thread_ts=1717610381.685139&cid=C02U025CVU4
• https://consensys.slack.com/archives/C02U025CVU4/p1722369858678939?thread_ts=1722019463.898419&cid=C02U025CVU4
• https://consensys.slack.com/archives/CBW7S9FSN/p1695241220126889?thread_ts=1695228967.143599&cid=CBW7S9FSN
• Edit: https://consensys.slack.com/archives/C02U025CVU4/p1732190183284219
• https://consensys.slack.com/archives/C02U025CVU4/p1728496388117329 (also linked in PR description above)

If our internal developers (myself included) have this much trouble with the current instructions for this step I don't think the situation will be much better for external contributors.

In the end I guess this comes down to the question of what's a higher priority for a readme: being correct or being helpful.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 54.54%. Comparing base (b0ef1a7) to head (385a936).
Report is 52 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #11714      +/-   ##
==========================================
+ Coverage   54.27%   54.54%   +0.27%     
==========================================
  Files        1711     1743      +32     
  Lines       38712    39317     +605     
  Branches     4738     4870     +132     
==========================================
+ Hits        21010    21446     +436     
- Misses      16253    16376     +123     
- Partials     1449     1495      +46     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[MajorLift]

Closing as wont-fix.