check for the latest version with new option #974

Merged
merged 1 commit into from Dec 29, 2016

Projects

None yet

3 participants

@tamgrosser
Contributor

wip @grosser

@presidentbeef
Owner

Hi,

Thank you for putting this together. I think it would be better to use the Gem interface though, in case of API changes: http://www.rubydoc.info/stdlib/rubygems/Gem.latest_version_for

@grosser
Contributor
grosser commented Dec 15, 2016

update!
a few observations:

  • tests are in a weird folder and do not end in _test.rb ... makes finding/working with them hard
  • tests cannot run on their own ... makes testing in isolation hard
  • there are no tests for commandline option handling in bin/brakeman
@presidentbeef
Owner

Thanks, this looks good.

  • The tests are in test/tests/ and used to also have test_... in the name and I couldn't take the triple redundancy.
  • You can run individual tests with ruby test/test.rb --name ...
  • Yes, this is true and has come up before and someone said they'd contribute testing for it...
@grosser
Contributor
grosser commented Dec 15, 2016

Anything needed for merge ?

@presidentbeef
Owner

Seems to work fine.

A couple minor things:

  • This should have a specific exit code like the -z
  • The option should be up around -z not in the scanning options section
@tamgrosser @grosser tamgrosser check for the latest version with new option 805ff43
@grosser
Contributor
grosser commented Dec 19, 2016

updated, looks better ?

@presidentbeef presidentbeef merged commit 3d93b06 into presidentbeef:master Dec 29, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@presidentbeef
Owner

Thanks!

@grosser grosser deleted the tamgrosser:latest branch Feb 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment