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

Include a binstubs check before running CI script #133

Merged
merged 1 commit into from Aug 1, 2015

Conversation

cupakromer
Copy link
Member

Newer RSpec contributors may not have setup binstubs. When running
script/run_build locally it errors in confusing ways. Often at this
point they are unsure how to properly get the binstubs onto the system.

This includes a very simplistic check for the non-optional binstubs. It
aggregates the missing binstubs and the associated gems, then provides
the user helpful options on how to get the binstubs.

While talking with the users it was decided that this is a kinder
approach than simply forcing the creating of the binstubs. Better to
inform the user then forcefully inject executables into their system.

return $success
}

check_binstubs
Copy link
Member

Choose a reason for hiding this comment

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

Why are you calling this here? The check below in run_build makes sense but here it seems odd.

Copy link
Member Author

Choose a reason for hiding this comment

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

o_O copy-paste error

@myronmarston
Copy link
Member

👍 Great idea! LGTM apart from the comment above.

Newer RSpec contributors may not have setup binstubs. When running
`script/run_build` locally it errors in confusing ways. Often at this
point they are unsure how to properly get the binstubs onto the system.

This includes a very simplistic check for the non-optional binstubs. It
aggregates the missing binstubs and the associated gems, then provides
the user helpful options on how to get the binstubs.

While talking with the users it was decided that this is a kinder
approach than simply forcing the creating of the binstubs. Better to
inform the user then forcefully inject executables into their system.
cupakromer added a commit that referenced this pull request Aug 1, 2015
Include a binstubs check before running CI script
@cupakromer cupakromer merged commit 5098f79 into master Aug 1, 2015
@cupakromer cupakromer deleted the check-binstubs-on-build branch August 1, 2015 04:38
@myronmarston
Copy link
Member

Looks like this breaks our build :(.

rspec/rspec-support#231

@cupakromer
Copy link
Member Author

Looks like rspec-support needs a custom function for this. I'll look into it tomorrow.

@myronmarston
Copy link
Member

I see why...rspec-support doesn't use cucumber. The check for bin/cucumber fails. You could use the check for the features directory that we already use:

if [ -d features ]; then

@cupakromer
Copy link
Member Author

NVM this needs a similar check:

  if [ -d features ]; then

@cupakromer
Copy link
Member Author

Updated in #25. Updated the other PRs and will wait for green properly 😬

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.

None yet

2 participants