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

Handle gems with dots in their name #65

Merged
merged 4 commits into from
Oct 24, 2019
Merged

Conversation

orien
Copy link
Member

@orien orien commented Oct 23, 2019

Context

The gem change annotation process fails when encountering a gem with a . in its name.

Octokit::UnprocessableEntity: POST https://api.github.com/repos/envato/marketplace/pulls/25214/comments: 422 - Invalid request.
--
  |  
  | No subschema in "oneOf" matched.
  | For 'properties/position', nil is not an integer.
  | "in_reply_to" wasn't supplied.
  | "position" is not a permitted key.
  | "line" wasn't supplied. // See: https://developer.github.com/v3/pulls/comments/#create-a-comment
  | /usr/local/bundle/gems/octokit-4.14.0/lib/octokit/response/raise_error.rb:16:in `on_complete'
  | /usr/local/bundle/gems/faraday-0.17.0/lib/faraday/response.rb:9:in `block in call'
  | /usr/local/bundle/gems/faraday-0.17.0/lib/faraday/response.rb:61:in `on_complete'
  | /usr/local/bundle/gems/faraday-0.17.0/lib/faraday/response.rb:8:in `call'
  | /usr/local/bundle/gems/octokit-4.14.0/lib/octokit/middleware/follow_redirects.rb:73:in `perform_with_redirection'
  | /usr/local/bundle/gems/octokit-4.14.0/lib/octokit/middleware/follow_redirects.rb:61:in `call'
  | /usr/local/bundle/gems/faraday-0.17.0/lib/faraday/request/retry.rb:130:in `call'
  | /usr/local/bundle/gems/faraday-0.17.0/lib/faraday/rack_builder.rb:143:in `build_response'
  | /usr/local/bundle/gems/faraday-0.17.0/lib/faraday/connection.rb:387:in `run_request'
  | /usr/local/bundle/gems/faraday-0.17.0/lib/faraday/connection.rb:175:in `post'
  | /usr/local/bundle/gems/sawyer-0.8.2/lib/sawyer/agent.rb:94:in `call'
  | /usr/local/bundle/gems/octokit-4.14.0/lib/octokit/connection.rb:156:in `request'
  | /usr/local/bundle/gems/octokit-4.14.0/lib/octokit/connection.rb:28:in `post'
  | /usr/local/bundle/gems/octokit-4.14.0/lib/octokit/client/pull_requests.rb:213:in `create_pull_request_comment'
  | /usr/local/bundle/gems/unwrappr-0.3.3/lib/unwrappr/github/pr_sink.rb:17:in `annotate_change'
  | /usr/local/bundle/gems/unwrappr-0.3.3/lib/unwrappr/lock_file_annotator.rb:60:in `block (2 levels) in annotate'
  | /usr/local/bundle/gems/unwrappr-0.3.3/lib/unwrappr/lock_file_diff.rb:19:in `block in each_gem_change'
  | /usr/local/bundle/gems/unwrappr-0.3.3/lib/unwrappr/lock_file_diff.rb:18:in `each'
  | /usr/local/bundle/gems/unwrappr-0.3.3/lib/unwrappr/lock_file_diff.rb:18:in `each_gem_change'
  | /usr/local/bundle/gems/unwrappr-0.3.3/lib/unwrappr/lock_file_annotator.rb:57:in `block in annotate'
  | /usr/local/bundle/gems/unwrappr-0.3.3/lib/unwrappr/github/pr_source.rb:20:in `block in each_file'
  | /usr/local/bundle/gems/unwrappr-0.3.3/lib/unwrappr/github/pr_source.rb:19:in `each'
  | /usr/local/bundle/gems/unwrappr-0.3.3/lib/unwrappr/github/pr_source.rb:19:in `each_file'
  | /usr/local/bundle/gems/unwrappr-0.3.3/lib/unwrappr/lock_file_annotator.rb:56:in `annotate'
  | /usr/local/bundle/gems/unwrappr-0.3.3/lib/unwrappr/lock_file_annotator.rb:39:in `annotate_github_pull_request'
  | /usr/local/bundle/gems/unwrappr-0.3.3/lib/unwrappr/cli.rb:34:in `execute'
  | /usr/local/bundle/gems/clamp-1.3.1/lib/clamp/command.rb:66:in `run'
  | /usr/local/bundle/gems/clamp-1.3.1/lib/clamp/subcommand/execution.rb:18:in `execute'
  | /usr/local/bundle/gems/clamp-1.3.1/lib/clamp/command.rb:66:in `run'
  | /usr/local/bundle/gems/clamp-1.3.1/lib/clamp/command.rb:140:in `run'
  | /usr/local/bundle/gems/unwrappr-0.3.3/exe/unwrappr:11:in `<top (required)>'
  | /usr/local/bundle/bin/unwrappr:23:in `load'
  | /usr/local/bundle/bin/unwrappr:23:in `<top (required)>'

It seems it couldn't determine the diff line number on which to add the annotation.

Change

Support annotating gem changes, where the gem name includes a ..

@@ -63,7 +63,7 @@ def extract_gem_and_change_type(line)
# '+ websocket-driver (0.6.5)'
# Careful not to match this (note the wider indent):
# '+ websocket-extensions (>= 0.1.0)'
pattern = /^(?<change_type>[\+\-]) (?<gem_name>[\w-]+) \(\d/
pattern = /^(?<change_type>[\+\-]) (?<gem_name>[\S-]+) \(\d/
Copy link
Member Author

Choose a reason for hiding this comment

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

Rather than expecting the gem name to consist of only word characters ([a-zA-Z0-9_]), expect it to be a string of non-whitespace characters ([^ \t\r\n\f\v]).

Copy link
Contributor

@johnsyweb johnsyweb left a comment

Choose a reason for hiding this comment

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

Thanks @orien .

@orien orien merged commit 43147bd into master Oct 24, 2019
@orien orien deleted the gems-with-dots-in-their-name branch October 24, 2019 01:29
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.

2 participants