Generalize the gem backtrace exclusion. #87
Conversation
|
This is just for our gems right? If so LGTM. |
| @@ -25,7 +25,7 @@ def ==(other) | |||
| # To work around JRuby error: | |||
| # TypeError: $stderr must have write method, RSpec::StdErrSplitter given | |||
| def write(line) | |||
| if line !~ /^\S+gems\/ruby\-\S+:\d+: warning:/ | |||
| if line !~ %r{^\S+/gems/\S+:\d+: warning:} | |||
There was a problem hiding this comment.
So it looks like previously ruby was in the path but it's not any more? That makes it look like there will be paths that were matched before that won't be now. Can this be made to support both?
Also, for regexes like this, I'm a big fan of using rubular to create a permalink with an example that I embed in a comment. Example:
if line !~ %r{^\S+/gems/\S+:\d+: warning:} # http://rubular.com/r/cWWgOu3WTJ
# ...
endMakes it easy to see what cases you were thinking of when you wrote the regex for later when you come back to it.
Previous regex matcher let gems slip through if a custom path was provided to bundler (e.g. `bundle install --path ../bundle`). Remove any explicit path dependencies on potential ruby version based directory names.
|
This should just be for our gems. I've added a rubular permalink, which includes comments with links for the failing build, and the previous regex which didn't match. |
|
This was intended to match ruby gems folders as formatted by default, your changes will potentially mean it includes any dir with gems in the name no? |
|
It matches paths which have a directory named |
|
👍 |
|
@JonRowe any further concerns? On my system the two general paths I use look like:
Both of those end in |
|
|
Generalize the gem backtrace exclusion.
Previous regex matcher let gems slip through if a custom path was
provided to bundler (e.g.
bundle install --path ../bundle). Remove anyexplicit path dependencies on potential ruby version based directory
names.