Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: rebase check in merge-pr script #28250

Closed

Conversation

IgorMinar
Copy link
Contributor

@IgorMinar IgorMinar commented Jan 19, 2019

Adds a check to verify that each PR branch to be merged upstream contains SHAs of commits that significantly changed our CI infrastructure.

This check is used to enforce that we don't merge PRs that have not been rebased recently and could result in merging of non-approved or otherwise bad changes.

@IgorMinar IgorMinar requested a review from a team as a code owner January 19, 2019 08:46
@IgorMinar IgorMinar added state: WIP area: build & ci Related the build and CI infrastructure of the project labels Jan 19, 2019
@ngbot ngbot bot added this to the needsTriage milestone Jan 19, 2019
@IgorMinar IgorMinar force-pushed the ci/rebase-check-in-merge-script branch 2 times, most recently from da32f42 to 4c3df08 Compare January 22, 2019 01:33
Adds a check to verify that each PR branch to be merged upstream contains SHAs of commits that significantly changed our CI infrastructure.

This check is used to enforce that we don't merge PRs that have not been rebased recently and could result in merging of non-approved or otherwise bad changes.
@IgorMinar IgorMinar force-pushed the ci/rebase-check-in-merge-script branch from 9fb3c2e to 40c4463 Compare January 22, 2019 06:42
@IgorMinar IgorMinar changed the title WIP: rebase check in merge-pr script ci: rebase check in merge-pr script Jan 22, 2019
@IgorMinar IgorMinar added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: WIP labels Jan 22, 2019
@IgorMinar IgorMinar requested a review from alxhub January 22, 2019 06:43
@IgorMinar IgorMinar added the target: patch This PR is targeted for the next patch release label Jan 22, 2019
@IgorMinar
Copy link
Contributor Author

I manually verified that the script does the right thing in all the common scenarios.

@IgorMinar IgorMinar added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jan 22, 2019
#
# This check is used to enforce that we don't merge PRs that have not been rebased recently and could result in merging
# of non-approved or otherwise bad changes.
REQUIRED_BASE_SHA_MASTER="3fba6eff79a9b50909199eaa4ebf754c1c4adba6" # pullapprove => CODEOWNERS migration
Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, we could have these as upstream branch pointers (or tags), which wouldn't require a commit in order to change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the thing I like about this approach is that we have an audit trail. tags don't require PRs or approvals, unless we go with "annotated tags" which might do the trick, I'm just not that familiar with them

@alxhub alxhub closed this in 9dabeea Jan 22, 2019
alxhub pushed a commit that referenced this pull request Jan 22, 2019
Adds a check to verify that each PR branch to be merged upstream contains SHAs of commits that significantly changed our CI infrastructure.

This check is used to enforce that we don't merge PRs that have not been rebased recently and could result in merging of non-approved or otherwise bad changes.

PR Close #28250
ngfelixl pushed a commit to ngfelixl/angular that referenced this pull request Jan 28, 2019
Adds a check to verify that each PR branch to be merged upstream contains SHAs of commits that significantly changed our CI infrastructure.

This check is used to enforce that we don't merge PRs that have not been rebased recently and could result in merging of non-approved or otherwise bad changes.

PR Close angular#28250
vetom pushed a commit to vetom/angular that referenced this pull request Jan 31, 2019
Adds a check to verify that each PR branch to be merged upstream contains SHAs of commits that significantly changed our CI infrastructure.

This check is used to enforce that we don't merge PRs that have not been rebased recently and could result in merging of non-approved or otherwise bad changes.

PR Close angular#28250
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: build & ci Related the build and CI infrastructure of the project cla: yes target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants