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

Gitlab integration: comments are duplicated because of default 20-item pagination #58

Merged
merged 1 commit into from
Jan 29, 2015

Conversation

siebertm
Copy link
Contributor

I fixed a small issue which leads to duplicate comments on when using the GitlabFormatter: When you have more than 20 comments on a commit, pagination kicks in and not all comments are seen, thus duplicate checking breaks.

I just set per_page to 500 to quickly work around that issue.

@mmozuras
Copy link
Member

@jeroenj could you take a look?

@jeroenj
Copy link
Contributor

jeroenj commented Jan 29, 2015

Nice find. 500 seems like a sensible default. I'd accept it. :)

@siebertm
Copy link
Contributor Author

Just a random number from the top of my head ;-)

@mmozuras
Copy link
Member

@siebertm could you add an entry to CHANGELOG?

@siebertm siebertm force-pushed the fix-gitlab-comment-duplication branch from beadcf5 to 9df5367 Compare January 29, 2015 16:15
This leads to duplicate comments. The ideal way would be to paginate
through all comments (with link headers), but the Gitlab gem doesn't allow
this currently.
@siebertm siebertm force-pushed the fix-gitlab-comment-duplication branch from 9df5367 to c09bbdf Compare January 29, 2015 16:16
mmozuras added a commit that referenced this pull request Jan 29, 2015
Gitlab integration: comments are duplicated because of default 20-item pagination
@mmozuras mmozuras merged commit b1e0cc9 into prontolabs:master Jan 29, 2015
@mmozuras
Copy link
Member

Released with this fix as 0.4.1. Thanks! 😄

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.

3 participants