Skip to content

Conversation

@jamesnaftel
Copy link
Contributor

Here is the first draft. There are a couple of things (like the token request) that I expect to change.

Expected changes:

  • Need to request a token
  • YB_COHORT_COUNT, YB_TEST_COHORT, rename to "YOURBASE_"
  • Add s3 bucket secret support

Copy link
Contributor

@zombiezen zombiezen left a comment

Choose a reason for hiding this comment

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

Looks like a good start to me. I had a few suggestions on organization and formatting, but I will defer to @clausti for technical accuracy.

Comment on lines 14 to 17
YOURBASE_DOWNLOAD_TOKEN=<enter token>
bundle add yourbase-rspec --git "https://$YOURBASE_DOWNLOAD_TOKEN:x-oauth-basic@github.com/yourbase/yourbase-rspec-skipper-engine.git"

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.

I recommend following the advice in https://www.vidarholen.net/contents/blog/?p=958:

Suggested change
YOURBASE_DOWNLOAD_TOKEN=<enter token>
bundle add yourbase-rspec --git "https://$YOURBASE_DOWNLOAD_TOKEN:x-oauth-basic@github.com/yourbase/yourbase-rspec-skipper-engine.git"
bundle install
bundle add yourbase-rspec --git "https://${YOURBASE_DOWNLOAD_TOKEN?}:x-oauth-basic@github.com/yourbase/yourbase-rspec-skipper-engine.git" &&
bundle install

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like this! Very clear.

1 examples, 0 failures, 1 skipped with YourBase🚀
```

To get additional details, please use -f, --formatter [p]rogress | [d]ocumentation]
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like it's supposed to include a documentation link?

@jamesnaftel jamesnaftel changed the title First draft of ruby instructions Update Quick Start docs May 3, 2021
Copy link
Member

@clausti clausti left a comment

Choose a reason for hiding this comment

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

Gotta go to a scheduled thing but will come back to this in a couple hours.

```

Run your tests with the same command you typically use. You should see a rocket ship at the beginning the RSpec test section.

Copy link
Member

Choose a reason for hiding this comment

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

We should have a section about Spring here, because it's the most common reason they won't see the rocketship.

Right now, they need to run spring stop from the command line to turn it off, but it would be better if we could figure out if manually requiring yourbase-rspec at the top of the spec_helper.rb gets around the incompatibility, so that we could tell them to do that instead of having to stop Spring. Stopping Spring is definitely fine in the CI but is a little kludgier for local dev where Spring does actual useful things, including shorten the spin-up time for "loading the rails environment" for specs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. Have this in my notes but wanted to try it out first. Will add it.

Copy link
Member

Choose a reason for hiding this comment

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

I think for now it is sufficient to say that if you do not see the rocketship but ARE running Spring, that it should be turned off to accelerate the tests.

Copy link
Member

@clausti clausti left a comment

Choose a reason for hiding this comment

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

This is in draft form, so I'm not really sure if you were looking for an "approval" or not yet?

@jamesnaftel
Copy link
Contributor Author

Thx for the feedback @clausti!

@jamesnaftel jamesnaftel marked this pull request as ready for review May 12, 2021 13:58
@jamesnaftel
Copy link
Contributor Author

For the email, I added "support@yourbase.io". We also have "ruby@yourbase.io". I was torn between the two. Happy to discuss.

Comment on lines 10 to 17
1. Add `gem 'yourbase-rspec', '~> 0.5.6',` to your Gemfile. If you have a `:test` group, add it there.
2. `bundle install` from the command line.
3. If you are not in a Rails project, you will also need to `require 'yourbase-rspec` your spec_helper (or at the top of the spec file you want to run).

Once you have a token, simply follow the steps below:
```sh
bundle add yourbase-rspec --git "https://${YOURBASE_DOWNLOAD_TOKEN?}:x-oauth-basic@github.com/yourbase/yourbase-rspec-skipper-engine.git" && bundle install
```
Copy link
Member

Choose a reason for hiding this comment

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

sorry if I missed this in the first pass, but I think the 1,2,3 and the sh section below it assume different things about the the public/privateness of the repo.

If we're going to publish this today-ish for Gusto purposes, we should make step 1. include the token syntax,
gem 'yourbase-rspec', '~> 0.6.0, git: "https://yourbase-tokenbot:#{ENV.fetch(YOURBASE_DOWNLOAD_TOKEN){nil}}@github.com/yourbase/yourbase-rspec-skipper-engine", branch: 'main', and then lines 14-16 are redundant and can be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed! Missed this as well. I think we should tune this for our launch which I believe means removing the "token" section. I'll update based on this assumption. We can always add it back.

jamesnaftel and others added 2 commits May 20, 2021 08:35
Co-authored-by: Christina Austin <4240737+clausti@users.noreply.github.com>
@jamesnaftel jamesnaftel requested a review from clausti May 20, 2021 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants