Alpaka: activate required review

Created on 24 May 2018  路  6Comments  路  Source: alpaka-group/alpaka

@ComputationalRadiationPhysics/alpaka-maintainers I activated for the dev and master branch that at least one review of a maintainer is required before a PR can be merged.

Question

All 6 comments

I activated:

  • Dismiss stale pull request approvals when new commits are pushed
    New reviewable commits pushed to a branch will dismiss pull request review approvals.
  • Require review from Code Owners
    Require an approved review in pull requests including files with a designated code owner.
  • Include administrators Enforce all configured restrictions for administrators.

I had the impression that at least the required review per PR had already been enabled some time before. Either my memory is wrong or it has been disabled somehow.
However, it does not matter now and you have reenabled everything. So I think everybody has recognized this change and the ticket can be closed.

I will for now disable the additional Require review from Code Owners rule because with this option, only those with write access to the repo can really approve a PR. Currently only admins have write access (because nobody else needs it). So the people which can approve a PR seem to be too limited (especially when someone is in holidays).
We can still reenable this option, but then we would need to extend the number of people with write access which I do not see necessary.

That's exactly the idea. Everyone can review (& approve/comment/request changes) but at least a maintainer needs to approve as well. When others take a look, @theZiz and me should be able to give the final approve as long as @psychocoderHPC is on holidays.

I have now reenabled the Require review from Code Owners option but gave the @ComputationalRadiationPhysics/alpaka-developers write access to the repo. So they can now all approve and Merge Pull Requests.

With great power comes great responsibility...

Was this page helpful?
0 / 5 - 0 ratings

Related issues

BenjaminW3 picture BenjaminW3  路  3Comments

tdd11235813 picture tdd11235813  路  4Comments

ax3l picture ax3l  路  5Comments

theZiz picture theZiz  路  5Comments

jkelling picture jkelling  路  4Comments