Skip to content

Changes to onboarding process #6560

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.

Sign up for GitHub

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

Merged
kbighorse merged 10 commits into from
Aug 9, 2022
Merged

Changes to onboarding process #6560

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e66529d
Install mailcatcher via Gemfile
kbighorse Jul 7, 2022
0246e9c
Step 4 runs `rake db:create` so Step 3 is redundant
kbighorse Jul 7, 2022
b1e54d6
Updating README with macOS openssl instructions
darth-cheney Jul 8, 2022
a6b6843
Target openssl v.3
kbighorse Jul 8, 2022
eba869f
Restore manual install of mailcatcher for now
kbighorse Jul 8, 2022
ec495cf
Switchin required openssl version to 1.1
darth-cheney Jul 11, 2022
0dbab6e
Merge branch 'main' into kbighorse/onboarding
kbighorse Jul 26, 2022
95e77b4
Ensuring we build js before testing and updating README
darth-cheney Jul 26, 2022
603e276
Merge branch 'main' into kbighorse/onboarding
kbighorse Aug 8, 2022
47562f2
Merge branch 'main' into kbighorse/onboarding
kbighorse Aug 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,11 @@ public/packs/manifest.json: yarn.lock $(shell find app/javascript -type f) ## Bu

test: export RAILS_ENV := test
test: $(CONFIG) ## Runs RSpec and yarn tests in parallel
bundle exec rake parallel:spec && yarn test
bundle exec rake parallel:spec && yarn build && yarn test

test_serial: export RAILS_ENV := test
test_serial: $(CONFIG) ## Runs RSpec and yarn tests serially
bundle exec rake spec && yarn test
bundle exec rake spec && yarn build && yarn test

fast_test: export RAILS_ENV := test
fast_test: ## Abbreviated test run, runs RSpec tests without accessibility specs
Expand Down
15 changes: 7 additions & 8 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,12 @@ We recommend using [Homebrew](https://brew.sh/), [rbenv](https://github.com/rben
-- (to install Node.js v.14 using brew: `brew install node@14`)
- [Yarn](https://yarnpkg.com/en/)
- [chromedriver](https://formulae.brew.sh/cask/chromedriver)

2. You will need to install openssl version 1.1:

- Run `brew install openssl@1.1`

2. Test that you have Postgres and Redis running.
3. Test that you have Postgres and Redis running.

For example, if you've installed with Homebrew, you can start the services like this:

Expand All @@ -46,13 +50,6 @@ We recommend using [Homebrew](https://brew.sh/), [rbenv](https://github.com/rben
$ brew services list
```

3. Create the development and test databases:

```
$ psql -c "CREATE DATABASE identity_idp_development;"
$ psql -c "CREATE DATABASE identity_idp_test;"
```

4. Run the following command to set up your local environment:

```
Expand All @@ -62,6 +59,8 @@ We recommend using [Homebrew](https://brew.sh/), [rbenv](https://github.com/rben
This command copies sample configuration files, installs required gems
and sets up the database. Check out our Makefile commands to learn more about what this command does: https://github.com/18F/identity-idp/blob/main/Makefile

Note: If you didn't explicitly install `openssl@1.1` in Step 2 above and you use a M1 Mac, you may see an error on this step. Homebrew works differently on a M1 Mac, so specifying the version is necessary for the make script to work, but may still work on x86.

5. Now that you have you have everything installed, you can run the following command to start your local server:

```
Expand Down
2 changes: 1 addition & 1 deletion bin/setup
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ Dir.chdir APP_ROOT do
run "bundle check || bundle install --without deploy production"
run "yarn install"
run "gem install thin -v 1.5.1 -- --with-cflags=\"-Wno-error=implicit-function-declaration\""
run "gem install mailcatcher -- --with-cppflags=-I/usr/local/opt/openssl/include"
Comment on lines 58 to -59
Copy link
Contributor

@zachmargolis zachmargolis Jul 7, 2022

Choose a reason for hiding this comment

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

I feel like we're always running into issues with new folks installing things with the right flags, is there a way to keep these flag instructions in the Gemfile? slack link

Copy link
Contributor

@eric-gade eric-gade Jul 8, 2022

Choose a reason for hiding this comment

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

I have two questions about this:

  1. Is it required that (mac) users install openssl via brew? I assume this is because of the lack of header files in the default install if so
  2. Homebrew has changed its directory structure for M1 macs (learned this yesterday). Perhaps we can just case on the system information and provide the correct flag in each case?

Copy link
Contributor

@zachmargolis zachmargolis Jul 8, 2022

Choose a reason for hiding this comment

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

  1. I am not sure! I feel it's one of those "just copy whatever somebody else said to make it work" kinda situations for me
  2. Yes, M1 Macs are going to have to do things differently, my computer is x86 so I have no firsthand experience with this yet 😬

But don't let that stop cleaning this up! I do think it's better to have these in the Gemfile than this script

Copy link
Contributor Author

@kbighorse kbighorse Jul 8, 2022

Choose a reason for hiding this comment

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

+1 for moving to Gemfile. I did hit dependency errors though with all recent versions of mailcatcher :/. (Both Eric and I use M1s)

Copy link
Contributor

@mitchellhenke mitchellhenke Jul 8, 2022

Choose a reason for hiding this comment

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

I think this stems from the author recommending against installing Mailcatcher via bundler: https://mailcatcher.me/#bundler

Conflicting versions strikes me as a potential problem, but I'd be fine with it as long as there are no conflicts.

Copy link
Contributor Author

@kbighorse kbighorse Jul 8, 2022

Choose a reason for hiding this comment

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

@pauldoomgov, @mitchellhenke mentioned you might have a M1 too? Have you run into this?

Copy link
Contributor Author

@kbighorse kbighorse Jul 8, 2022

Choose a reason for hiding this comment

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

@mitchellhenke you're right, I forgot about that. I introduced that problem, even though we still have the original problem.

Copy link
Contributor Author

@kbighorse kbighorse Jul 8, 2022

Choose a reason for hiding this comment

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

an observation: the versions of mailcatcher and thin that this installs are from 2011 and 2013, respectively. I think I'm going to try and get current versions working and keep it out of bundler.

Copy link
Contributor

@aduth aduth Jul 8, 2022

Choose a reason for hiding this comment

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

Do we even need thin ? 🤔

Copy link
Contributor

@zachmargolis zachmargolis Jul 8, 2022

Choose a reason for hiding this comment

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

I think mailcatcher needs thin... which gives me another idea 💡 what if we had Gemfile.mailcatcher and updated our scripts to use that completely separately?

BUNDLE_GEMFILE=Gemfile.mailcatcher bundle exec mailcatcher

# Gemfile.mailcatcher

gem 'mailcatcher'

run "gem install mailcatcher -- --with-cppflags=-I$(brew --prefix openssl@1.1)/include"

puts "\n== Preparing database =="
run 'bin/rake db:create'
Expand Down