-
Notifications
You must be signed in to change notification settings - Fork 9
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
Add tests, handle ruby engine / engine_version #2
Conversation
bats | ||
rbenv | ||
|
||
# Editor temp/backup files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you take this part out? People can configure their own extra editor files in their global .gitconfig.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the reminder - replaced with:
# See: https://gist.github.com/ianheggie/9327010
# for Global git ignore for OS/IDE/temp/backup files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer not to have that in at all; I don't think this needs to link to a gist to tell people how to ignore files in git.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer it in my repo - just take it out in yours
@@ -28,5 +30,13 @@ to make this easier. Once you have rbenv-aliases installed: run: | |||
|
|||
Caveats | |||
------- | |||
The logic currently used to find the version is pretty dumb, and it only supports simple lines | |||
like `ruby '2.0.0'`. Other interpreters are not supported at this point. | |||
The logic currently used to find the version is simplistic: it supports |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this reads better as s/simplistic: it supports/simplistic; it supports:/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took your input, read up a bit, and extended it to simplistic; rbenv-aliases supports:
so both sides could stand alone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rbenv-aliases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opss - fixed (was swapping back and forth between)
Thanks @ianheggie, this is awesome! I've added a bunch of suggestions so the simplicity of the test code is a bit more proportional to the production code. |
I have made the final changes I mentioned above, and the tests pass - I hope you are happy with the end result, but whatever you choose, I am going to leave this branch as is now. I am investigating on a separate branch ( hook_into_version ) about getting the version* commands to work with Gemfiles. I will probably also change the algorithm to pick the closest it finds out of Gemfile and .ruby-version (so a closer .ruby-version overrides a Gemfile and visa versa) - that will also be in a separate branch. [Sometime in the future, but not right now]
|
Thanks, do you mind if I squash your commits before merging? |
Handle ruby engines in Gemfile ruby line; Add changes suggested by aripollak; Add comment on handling ruby engines and that version commands are not updated in README.md; Add .travis.yml for travis CI testing and link in README.md; Add .gitignore for test artifacts;
opps ... did the squash, but forgot to send you a note ... you noticed it anyway (I squashed it my end to make it easy to bring future changes bach into mine). All the best. Ian |
👍 I did end up making quite a few cleanups, so you'll probably want to bring in my latest changes as well. |
This adds tests (for travis-ci.org - which pass),
and enhances the system to handle ruby engines.
(All which still just using sed regular expressions, so still fast).