PRCR - temporary disabled Audit rule

[Bullrich]

@the-right-joyce has requested to disable the Audit rule until we can fix the problem that it always request reviewers (even if the user belongs to the prevent-review-request field.

[alvicsam]

I think only this part can be commented:

      - min_approvals: 1
        teams:
          - srlabs

In this case it will require 2 distinct approvals from locks-review and polkadot-review teams for changes in paths:

• polkadot/runtime/(kusama|polkadot|common)/*
• polkadot/primitives/src/*.rs
• substrate/primitives/*
• substrate/frame/*

Excluding md files and weights.

@bkchr wdyt?

[bkchr]

Neither polkadot-review or locks-review should be required for general changes in FRAME. Before this was only required for changes to the Polkadot/Kusama runtime. This means, these checks are useless any way as we will move these runtimes out to the fellowship. So, we probably should only keep locks-review and they can operate on the entire code base for when "locked" code is changed.

[juangirini]

In this case it will require 2 distinct approvals from locks-review and polkadot-review teams for changes in paths:

• polkadot/runtime/(kusama|polkadot|common)/*
• polkadot/primitives/src/*.rs
• substrate/primitives/*
• substrate/frame/*

How important is to have a review from locks-review and polkadot-review on substrate/frame/*?

[Bullrich]

Should I change this code to be this then?

Suggested change

	# - name: Audit rules
	# check_type: changed_files
	# condition:
	# include: ^polkadot/runtime\/(kusama|polkadot|common)\/.*|^polkadot/primitives/src\/.+\.rs$|^substrate/primitives/.*|^substrate/frame/.*
	# exclude: ^polkadot/runtime\/(kusama|polkadot)\/src\/weights\/.+\.rs$|^substrate\/frame\/.+\.md$
	# all_distinct:
	# - min_approvals: 1
	# teams:
	# - locks-review
	# - min_approvals: 1
	# teams:
	# - polkadot-review
	# - min_approvals: 1
	# teams:
	# - srlabs
	- name: Audit rules
	check_type: changed_files
	condition:
	include: ^polkadot/runtime\/(kusama|polkadot|common)\/.*|^polkadot/primitives/src\/.+\.rs$|^substrate/primitives/.*|^substrate/frame/.*
	exclude: ^polkadot/runtime\/(kusama|polkadot)\/src\/weights\/.+\.rs$|^substrate\/frame\/.+\.md$
	all_distinct:
	- min_approvals: 1
	teams:
	- locks-review
	# - min_approvals: 1
	# teams:
	# - polkadot-review
	# - min_approvals: 1
	# teams:
	# - srlabs

[bkchr]

@Bullrich what is blocking this?

[Bullrich]

I updated the PR to remove frame and only have the lock-team. Let me know if I'm missing anything

[bkchr]

This is still wrong... locks-review was about protecting
🔒 🔒 🔒
line of code
🔒 🔒 🔒

[Bullrich]

What team should I set here?

So, we probably should only keep locks-review and they can operate on the entire code base for when "locked" code is changed.

We have miss interpreted your message, I thought you referred to using the lock-teams.

Should I simply remove this rule?

[mordamax]

well it just moved and merged all together from 3 repos + mixed with requirements from paritytech/pr-custom-review#136, there's high chance that it could be done better
Lets revisit these pr-cr settings in scope of monorepo holistically

so in order to implement properly paritytech/pr-custom-review#136 this task, with post-actions like adding a project etc we will need to wait until https://github.com/paritytech/review-bot/milestone/1 is done and we can integrate it here with all new rules

before it's done i think this whole condition from paritytech/pr-custom-review#136 could be removed for now, and keep only the old polkadot one https://github.com/paritytech/polkadot/blob/master/.github/pr-custom-review.yml#L10-L11

[bkchr]

Just remove the rules here entirely for now.

[mordamax]

@bkchr you mean all PRCR rules or only Audit?
if all - why?

[Bullrich]

Comment just to trigger PRCR again