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.

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

Jump to bottom

adds prompt for version script and a little more documentation #1729

Merged
merged 1 commit into from
May 17, 2016
Merged

adds prompt for version script and a little more documentation #1729

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 13 additions & 3 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,15 @@ Run a single test
`$ rake test TEST=path/to/test.rb TESTOPTS="--name=test_something"`

Run tests against different Rails versions by setting the RAILS_VERSION variable
and bundling gems.
and bundling gems. (save this script somewhere executable and run from top of AMS repository)
Copy link
Member

Choose a reason for hiding this comment

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

nice. I just wrote this as something to paste into the console.

I think a better solution to recommend something more robust would be to recommend using wwtd and just use the current Ruby wwtd --local

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dont know much about WWTD .... I was originally gonna run it in the console, but found it simpler just to ../ams_tester.sh and bail out when something crapped out 😩

Copy link
Member

@bf4 bf4 May 17, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

for example, this script is now out of date since we don't run on 4.0 anymore. wwtd just using the travis file as a source of truth. you could, of course do something like ruby -ryaml -e 'puts YAML.load_file("./.travis.yml")["env"]["matrix"].join(" ")' instead to make the list

Copy link
Member

Choose a reason for hiding this comment

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

wwtd sounds like a great idea. Also, pulling the versions from .travis.yml would be very nice.

Copy link
Member

Choose a reason for hiding this comment

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

(save this script somewhere executable and run from top of AMS repository)

can we make this even easier?


```bash
for version in 4.0 4.1 4.2 master; do
#!/usr/bin/env bash

rcommand='puts YAML.load_file("./.travis.yml")["env"]["matrix"].join(" ").gsub("RAILS_VERSION=", "")'
versions=$(ruby -ryaml -e "$rcommand")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bf4 looking up versions based on your yml suggestion


for version in ${versions[@]}; do
Copy link
Member

Choose a reason for hiding this comment

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

@cgmckeever I am envious of your bash skills. I've never seen that and it is awesome

export RAILS_VERSION="$version"
rm -f Gemfile.lock
bundle check || bundle --local || bundle
Expand All @@ -88,7 +93,12 @@ for version in 4.0 4.1 4.2 master; do
else
# red in ANSI
echo -e "\033[31m **** Tests failed against Rails ${RAILS_VERSION} **** \033[0m"
fi
read -p '[Enter] any key to continue, [q] to quit...' prompt
if [ "$prompt" = 'q' ]; then
unset RAILS_VERSION
exit 1
fi
Copy link
Contributor Author

@cgmckeever cgmckeever May 17, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

@remear totally understand your thoughts on leaving this auto-pilot. My only counter argument is, that this script is really nice to get a new contributor going - and jumping on errors right away (for me) was nice. As an experienced contributor, you probably have some more advanced tools in your belt (ie wwtd)) and/or can just comment out L96 to skip the prompt.

Copy link
Member

@bf4 bf4 May 17, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

agreed. but if we make it too nice, then it should just go in bin, right? and save the hassle of downloading it?

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 thought about this, but wasnt sure about a script floating around a gem, so I just left it in the docs

fi
unset RAILS_VERSION
done
```
Expand Down