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

A gerrit service. #305

Merged
merged 3 commits into from Apr 25, 2016
Merged

A gerrit service. #305

merged 3 commits into from Apr 25, 2016

Conversation

ralphbean
Copy link
Sponsor Collaborator

It works!

I noticed while doing this that the IssueService.validate_config call is still
non-idiomatic, fwiw.

@ryneeverett
Copy link
Collaborator

Did you intend to commit gerrit.py?

url = self.url + '/a/changes/' + \
'?q=is:open+is:reviewer' + \
'&o=MESSAGES&o=DETAILED_ACCOUNTS'
response = self.session.get(url)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not:

response = self.session.get(
    self.url,
    params={'q': 'is:open+is:reviewer', 'o': ['MESSAGES', 'DETAILED_ACCOUNTS']})

Copy link
Sponsor Collaborator Author

Choose a reason for hiding this comment

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

Ah, because requests will percent-encode the : characters, and gerrit doesn't like that. :(

@ralphbean
Copy link
Sponsor Collaborator Author

Did you intend to commit gerrit.py?

Aww... no. :( Time to change that pw.

@ralphbean
Copy link
Sponsor Collaborator Author

@ryneeverett, updated and rebased.

@ryneeverett
Copy link
Collaborator

LGTM

@ralphbean ralphbean merged commit cded48e into develop Apr 25, 2016
@ralphbean ralphbean deleted the feature/gerrit branch April 25, 2016 15:25
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.

None yet

2 participants