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.

Sign up for GitHub

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

Jump to bottom

PR Builder GitLab Integration #6066

Merged
merged 10 commits into from
Aug 19, 2019
Merged

PR Builder GitLab Integration #6066

merged 10 commits into from
Aug 19, 2019

Conversation

saadmk11
Copy link
Member

ref: #6052

@saadmk11 saadmk11 added the PR: work in progress Pull request is not ready for full review label Aug 11, 2019
@saadmk11 saadmk11 requested review from ericholscher and a team August 12, 2019 14:39
@saadmk11 saadmk11 removed the PR: work in progress Pull request is not ready for full review label Aug 12, 2019
ericholscher
ericholscher reviewed Aug 12, 2019
View reviewed changes
Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

This looks good. A few small nits, but I think it's close.

readthedocs/api/v2/views/integrations.py Outdated Show resolved Hide resolved
readthedocs/api/v2/views/integrations.py
return identifier, verbose_name

except KeyError:
raise ParseError('Parameters "id" and "iid" are required')
Copy link
Member

Choose a reason for hiding this comment

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

Where is this ParseError handled?

Copy link
Member Author

@saadmk11 saadmk11 Aug 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

When there is a ParseError we return 400 to the API. We do the same thing for the push events.

readthedocs.org/readthedocs/api/v2/views/integrations.py

Lines 468 to 472 in e53af66

try:
branches = [self._normalize_ref(data['ref'])]
return self.get_response_push(self.project, branches)
except KeyError:
raise ParseError('Parameter "ref" is required')

readthedocs/rtd_tests/tests/test_project.py Show resolved Hide resolved
saadmk11 and others added 3 commits August 13, 2019 09:18
@saadmk11 @ericholscher
Update readthedocs/api/v2/views/integrations.py
52c6ef9 
Co-Authored-By: Eric Holscher <25510+ericholscher@users.noreply.github.com>
@saadmk11
Tests updated
419cdb3
@saadmk11
Merge branch 'gitlab-integration' of github.com:saadmk11/readthedocs.…
4c89847 
…org into gitlab-integration
@saadmk11 saadmk11 mentioned this pull request Aug 16, 2019
Merged
ericholscher
ericholscher approved these changes Aug 19, 2019
View reviewed changes
Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

👍

@ericholscher ericholscher merged commit 3ccf7c3 into readthedocs:master Aug 19, 2019
@saadmk11 saadmk11 deleted the gitlab-integration branch August 19, 2019 18:55
@saadmk11 saadmk11 mentioned this pull request Aug 20, 2019
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericholscher ericholscher ericholscher approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@saadmk11 @ericholscher