search

haskell / security-advisories

https://haskell.github.io/security-advisories/
Other
46 stars 18 forks source link

Start requiring an approving review for mergers #175

Closed MangoIV closed 7 months ago

MangoIV commented 8 months ago

I think it would be good if we required an approving review before merging something, this is good practise for several reasons and should be clear.

blackheaven commented 7 months ago

We have no formal process, currently, whenever someone of the HSRT review a PR, (s)he can then merge it, without any particular deadline.

MangoIV commented 7 months ago

I think it would be good practice to always (or at least in the default case) require an approving review; even if the PR was raised by a team member, that way we make sure that the code has high quality and people really looked over the entire PR; especially to try and stay on-topic, two pairs of eyes always see more than one pair. ;)

blackheaven commented 7 months ago

I agree, for the record, aside of #168 which was approved on our internal ML, I only did it for CI related problems #172 #173 #174, which hard quite hard to debug on other repositories.

frasertweedale commented 7 months ago

I'm fine with Gautier self-merging CI-related changes :) But yeah, for library and executable code, and advisories, for sure, we should require reviews.