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

block pull requests to stable #6001

Merged
merged 5 commits into from
May 1, 2024
Merged

block pull requests to stable #6001

merged 5 commits into from
May 1, 2024

Conversation

etan-status
Copy link
Contributor

Pull requests should be opened against the unstable branch. See https://nimbus.guide/contribute.html#build-and-deploy

Pull requests should be opened against the `unstable` branch.
See https://nimbus.guide/contribute.html#build-and-deploy
@etan-status
Copy link
Contributor Author

Most recent instance: #6000

@arnetheduck
Copy link
Member

meh, or we can just retarget them as was done with 6000 instead of being obnoxious about it

@etan-status
Copy link
Contributor Author

The CI is not re-run on base change / it may show as green on stable, but actually fail after merge.
Failing allows quickly re-submitting directly. If it's a meaningful PR, it also may need resolving merge conflicts.

We can keep it pending until the next one that doesn't cleanly retarget :-) They happen from time to time.

Copy link

github-actions bot commented Feb 29, 2024

Unit Test Results

         9 files  ±0    1 322 suites  ±0   26m 38s ⏱️ - 10m 27s
  4 981 tests ±0    4 633 ✔️ ±0  348 💤 ±0  0 ±0 
20 826 runs  ±0  20 422 ✔️ ±0  404 💤 ±0  0 ±0 

Results for commit 464b6f9. ± Comparison against base commit e70fa6d.

♻️ This comment has been updated with latest results.

@tersec
Copy link
Contributor

tersec commented Mar 9, 2024

Yeah, it's always a gamble with retargeting, because it's either:

a) ask them to do it on their own, which is the human version of "obnoxious" here;
b) just try to target, and hope it goes okay without further intevention; because
c) failed version of (b) is bad, where tried to "just retarget them".

That one was a documentation fix so it was low-risk but typically for anything vaguely nontrivial I ask the person filing the PR to do it because of the risk of (c).

In favor of this.

@etan-status
Copy link
Contributor Author

@tersec
Copy link
Contributor

tersec commented Mar 20, 2024

@tersec
Copy link
Contributor

tersec commented Mar 23, 2024

One edge case here is Dependabot, which uses stable and might not be able to use any other branch except if it were the default (i.e. navigating to https://github.com/status-im/nimbus-eth2/ navigated to that branch instead): #6111

These are 100% of the time in the Nimbus codebase trivial changes, because thye're in Python dependencies, and just version bumps in requirements.txt and typically are rebased by GitHub automatically and easily.

@etan-status
Copy link
Contributor Author

There is an option to specify the target branch for Dependabot, but it is documented to be ignored for security patches: https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#target-branch

Therefore, have updated the action to not trigger on Dependabot triggered PRs.

@tersec
Copy link
Contributor

tersec commented Apr 18, 2024

#6219

@tersec
Copy link
Contributor

tersec commented Apr 29, 2024

#6253

@tersec tersec merged commit 9e8d2e7 into unstable May 1, 2024
13 checks passed
@tersec tersec deleted the dev/etan/ci-prtarget branch May 1, 2024 19:30
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