easybuilders / easybuild-framework

EasyBuild is a software installation framework in Python that allows you to install software in a structured and robust way.
https://easybuild.io
GNU General Public License v2.0
150 stars 202 forks source link

Unable to merge with --merge-pr as we get that the last test is FAILED even though this not being the most recent test report #3532

Open branfosj opened 3 years ago

branfosj commented 3 years ago
$ eb --merge-pr 11663 -D
== temporary log file in case of crash /scratch/branfosj-admin/eb-smzc4fug/easybuild-cwfh6o8b.logeasybuilders/easybuild-easyconfigs PR #11663 was submitted by boegel, you are using GitHub account 'branfosj'Checking eligibility of easybuilders/easybuild-easyconfigs PR #11663 for merging...
* targets develop branch: OK
* test suite passes: OK
* last test report is successful: FAILED => not eligible for merging!
* approved review: OK (by branfosj)
* milestone is set: OK (next release (4.3.3?))WARNING: Review indicates this PR should not be merged (use -f/--force to do so anyway)

We get returned some of the comments. I think the oldest 30 and in this case the most recent of those is a failed test report.

Quick fix (which will fail less often), add at https://github.com/easybuilders/easybuild-framework/blob/develop/easybuild/tools/github.py#L2197

parameters['per_page'] = 100

More complete fix: In the response headers when there are multiple pages we get:

link
<https://api.github.com/repositories/6228403/issues/11663/comments?per_page=10&page=2>;  rel="next",  <https://api.github.com/repositories/6228403/issues/11663/comments?per_page=10&page=4>;  rel="last"

These could be used to either get the last comnents or to get all, depending on what we want.

migueldiascosta commented 3 years ago

would be fixed by https://github.com/easybuilders/easybuild-framework/pull/2553 or https://github.com/easybuilders/easybuild-framework/pull/2776 (and with https://github.com/easybuilders/easybuild-framework/pull/2776#issuecomment-466909085, there are 3 alternatives :) )