Skip to content

Github action to require 2 reviewers#884

Merged
gakonst merged 1 commit intodevelopfrom
bwilson/gh-action-label-requires-reviews
May 20, 2021
Merged

Github action to require 2 reviewers#884
gakonst merged 1 commit intodevelopfrom
bwilson/gh-action-label-requires-reviews

Conversation

@optimisticben
Copy link
Contributor

Description
Update the labeler action to add a new label (2-reviewers) for sensitive paths.
Add a new action (require-reviewers) which fails unless a PR labeled with 2-reviewers has 2 or more reviews.

Additional context
Instead of the inverse match I went this route as this check need to a reviewer count higher then the repo settings.
The require-reviewers check needs to be made required on the develop branch for this action's intent to be enforced.

@changeset-bot
Copy link

changeset-bot bot commented May 14, 2021

⚠️ No Changeset found

Latest commit: 9b638bd

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions bot added the A-ci label May 14, 2021
@snario
Copy link
Contributor

snario commented May 18, 2021

@optimisticben can this be removed from Draft? If so I can do this:

The require-reviewers check needs to be made required on the develop branch for this action's intent to be enforced.

@optimisticben
Copy link
Contributor Author

@optimisticben can this be removed from Draft? If so I can do this:

The require-reviewers check needs to be made required on the develop branch for this action's intent to be enforced.

The action also requires a PERSONAL_ACCESS_TOKEN secret. I was hoping we already have an automation account for that purpose.

@optimisticben optimisticben force-pushed the bwilson/gh-action-label-requires-reviews branch from 8198649 to 812abcc Compare May 18, 2021 16:50
@optimisticben optimisticben marked this pull request as ready for review May 18, 2021 16:50
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added
image

@optimisticben optimisticben force-pushed the bwilson/gh-action-label-requires-reviews branch from 0c45863 to 9b638bd Compare May 19, 2021 18:07
@gakonst gakonst merged commit 765f4d1 into develop May 20, 2021
@gakonst gakonst deleted the bwilson/gh-action-label-requires-reviews branch May 20, 2021 12:08
InoMurko pushed a commit to omgnetwork/optimism that referenced this pull request May 25, 2021
theochap pushed a commit that referenced this pull request Dec 10, 2025
OptimismBot pushed a commit that referenced this pull request Feb 25, 2026
github-merge-queue bot pushed a commit that referenced this pull request Feb 25, 2026
* feat: l2cm impl l2contractsmanager (#837)

* feat: add initial iteration of L2ContractsManager

* feat: add network configuration structs

* feat: load full config for L2ContractsManager

* feat: implement L2CM::_apply

* feat: add gas price oracle

* refactor: move L2CM types to library

* fix: upgrade ProxyAdmin predeploy

* chore: enforce delegatecall for L2CM::upgrade

* feat: add conditional upgrade for CGT

* refactor: remove non-proxied predeploys

* chore: renamed l2cm

* refactor: l2cm address comments (#839)

* refactor: rename _fullConfig to _loadFullConfig to match OPCM v2

* chore: remove non-proxied weth from implementations struct

* test: add config preservation test

* test: add CGT specific tests

* refactor: avoid casting network config values to address

* test: add test cases

* chore: pr ready (#844)

* chore: remove unnecesary casting on L2CM

* feat: add interface for XForkL2ContractsManager

* chore: add natspec to XForkL2ContractsManager

* chore: pr ready

* refactor: moves util functions out of L2CM implementation (#848)

* feat: l2cm address comments (#850)

* chore: add comment clarifying use `useCustomGasToken`

* chore: upgrade both native native asset liquidity and liquidity controller predeploys together

* feat: prohibit downgrading predeploy implementations

* refactor: make isCustomGasToken part of the network full config

* fix: add missing import

* fix: use FeeVault legacy getters for backward compat

* chore: update name XForkL2ContractsManager to L2ContractsManager

* feat: conditionally skip some predeploys based on them being supported in a given chain (#857)

* fix: l2cm address comments (#872)

* chore: add todo tracking removal of L2ProxyAdmin skips

* chore: add natspec comment for isPredeployNamespace

* chore: use vm.prank(address,bool) to prank a delegatecall

* chore: add todo for dev flags for CrossL2Inbox and L2ToL2CrossDomainMessenger

* feat: allow immutables for L2CM in semgrep rules

* chore: pr ready

* test: L2CM verify testing (#874)

* test: add coverage test for predeploy upgrades

* chore: update test natspec

* chore: just pr ready

* chore: L2CM round comments (#877)

* refactor: move helper function into Predeploys.s.sol

* fix: add conditional deployer to L2CM

* chore: update to l1block and l1blockCGT

* test: fixes issue where OptimismSuperchainERC20 tests fail due to profile ambiguity

* chore: just pr ready

* chore: l2cm round comments2 (#883)

* fix: move code length check out of isUpgradeable

* chore: inline fullCofig_.isCustomGasToken initialization

* chore: add public getters for the implementations on the L2CM

* chore: remove XForkL2ContractsManager sol rule exclusion

* test: add downgrade prevention test suite

* chore: just pr ready

* refactor: check for address 0 instead code length

* Revert "refactor: check for address 0 instead code length"

This reverts commit 1fa8694.

* chore: remove non-needed check

* chore: remove unused function in tests (#884)

* refactor: l2cm group impls (#885)

* refactor: remove individual getters in favor of a unified one

* test: add test for getImplementations

* test: add OZ v5 Initializable compatibility in L2ContractsManagerUtils (#887)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants