[GitHub] [incubator-nuttx] antmerlino commented on pull request #3747: mm:initialize ensure alignment.

[GitBox <gi...@apache.org>]

antmerlino commented on pull request #3747:
URL: https://github.com/apache/incubator-nuttx/pull/3747#issuecomment-845134457


   > Yes, we need more test to avoid the regression. But, please give some test plan or progress in PR, so we can coordinate with each other.
   
   I think we need to give people more time. Everyone is dealing with their own sets of problems on a day to day basis. 3 days is an eternity when it's your work you're trying to push through - but it's not so long when you've got your own deadlines and then additional PRs come in that need thorough testing/review. 
   
   To @davids5 point, this is why it is probably better practice to not allow individuals from the same organization to approve PRs. Both will be incentivized to get it through. Much sophisticated engineering is done and cross-checked using red/blue teams. This mentality helps be critical of the others work. The same philosophy should be being applied here. 
   
   I really don't want to create processes that are burdensome and slow things down. However, I do think it's important that we adjust our process to avoid regressions like this and improve the security/reliability of the OS.
   
   What I would propose is this:
   
   1. Change 'Impact' in the PR template to be 'What boards will be affected by this change?'
       To this, common answers will be:        
           - All stm32h7 boards
           - All boards
           - Any board using XX driver 
   The reason for this question is to be able to quickly find PRs that will effect everyone. If the PR states it effects all boards, as this change would, we can add a tag to the PR. Those PRs should get a more thorough review.     
   
   2. Any PR that gets the tag, or is otherwise labeled a critical change should be reviewed by more than 1 person. Perhaps we say 2 is enough, or maybe it should be 3. In either case we should setup Github to require 2-3 accepted reviews before allowing a merge.
   
   3. In general, I think we should set a policy that individuals from the same organization cannot approve their own PRs.
   
   If this seems like a reasonable proposal, we can move the conversation to the dev list.
   
   Let me know what you guys think.
   
   -Anthony


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org