Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up environment-specific instructions in SETUP.md #46750

Merged
merged 2 commits into from
Jun 10, 2022

Conversation

Hamms
Copy link
Contributor

@Hamms Hamms commented Jun 8, 2022

Specifically:

  • The "general" instructions section had several M1-mac-specific instructions in it; move them into the mac-specific section
  • Add instructions for an Ubuntu 22.04-specific issue

Also, we were manually maintaining the numbers of the steps in code, which makes refactoring like this more tedious. By instead using a consistent notation, we can let Markdown handle the numbering for us. Because the numbering change is somewhat noisy, I recommend reviewing this PR commit-by-commit:

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

Hamms added 2 commits June 8, 2022 12:33
Specifically:

- The "general" instructions section had several M1-mac-specific instructions in it; move them into the mac-specific section
- Add instructions for an Ubuntu 22.04-specific issue

8. `bundle install`
1. `bundle install`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pretty sure we can't run bundle install on M1 without the lockfile changes.

However, I've got another PR that will hopefully remove this workaround.

Copy link
Contributor Author

@Hamms Hamms Jun 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but the directions for the lockfile changes are in the section referenced by this earlier step:

1. Install OS-specific prerequisites
   - See the appropriate section below: [OS X](#os-x-monterey---including-apple-silicon-m1), [Ubuntu](#ubuntu-1804-download-iso), [Windows](#windows)

This section of the instructions is for general instructions which apply to all operating system environments; OS-specific instructions should go in the OS-specific sections

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, i missed one of the 'expand' sections in the diff and it was hiding the M1 heading 🤦

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I should have also mentioned:

Some of the instructions I moved, but some of them I went to move and realized they already existed both in the place I was moving them from and the place I was moving them to, so I just removed them.


8. `bundle install`
1. `bundle install`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, i missed one of the 'expand' sections in the diff and it was hiding the M1 heading 🤦

@Hamms Hamms merged commit 292a33e into staging Jun 10, 2022
@Hamms Hamms deleted the clean-up-setup-ssl-instructions branch June 10, 2022 18:08
@Hamms Hamms mentioned this pull request Jul 18, 2023
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants