Skip to content

CONTRIBUTING: rebase master to staging without pinging people#166472

Merged
Mic92 merged 2 commits intoNixOS:masterfrom
Mic92:docs
Apr 4, 2022
Merged

CONTRIBUTING: rebase master to staging without pinging people#166472
Mic92 merged 2 commits intoNixOS:masterfrom
Mic92:docs

Conversation

@Mic92
Copy link
Member

@Mic92 Mic92 commented Mar 30, 2022

Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@Mic92 Mic92 changed the title CONTRIBUTING: document how to rebase master to staging without pingin… CONTRIBUTING: rebase master to staging without pinging people Mar 30, 2022
@dasJ
Copy link
Member

dasJ commented Mar 30, 2022

cc @jonringer since this is based on #144227 (review) ;)

@Mic92 Mic92 requested a review from SuperSandro2000 March 30, 2022 20:48
Co-Authored-By: Martin Weinelt <hexa@darmstadt.ccc.de>
Co-Authored-By: Janne Heß <janne@hess.ooo>
Copy link
Member

@Artturin Artturin left a comment

Choose a reason for hiding this comment

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

i've been using this made by jonringer

# finding the common merge base will avoid pinging all CODEOWNERs
common=$(git merge-base origin/master origin/staging)
git rebase --onto=$common HEAD~1 # or however many commits you want to pull
git push --force

now i wont have to dig up my notes everytime hehe

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Mar 30, 2022
commits=$(git merge-base $(git branch --show-current) upstream/master)
# Rebase all commits onto the common base
git rebase --onto=$common $commits
# Force push your changes
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Force push your changes
# Force push your changes
git fetch upstream staging
git rebase upstream/staging

Copy link
Contributor

Choose a reason for hiding this comment

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

this shouldn't be necessary since they should have done git rebase --onto=$common $commits

Copy link
Member

Choose a reason for hiding this comment

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

Rebasing onto upstream/staging should only be done after having force-pushed and switched the target branch in GitHub (see #166472 (comment)). (I may be misunderstanding your suggestion though)

Copy link
Member

Choose a reason for hiding this comment

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

The merge base between master and staging is usually older than staging itself, and in between that diff there may be conflicting changes still.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh yea, that's fair. Keep forgetting about divergence :(

@ajs124 ajs124 mentioned this pull request Mar 30, 2022
@K900
Copy link
Contributor

K900 commented Mar 31, 2022

Feels like this could be solved in ofborg instead, at least for the most common cases? Just have a list of valid branch targets (master, staging, release-XX.YY), then pick the one with lowest distance to merge base for the pings.

@dasJ
Copy link
Member

dasJ commented Mar 31, 2022

@K900 that's very much out of scope for this PR

@K900
Copy link
Contributor

K900 commented Mar 31, 2022

Oh yeah, definitely. Mostly just wanted to note down the idea because the current UX is... not great, even with this being documented.

@risicle risicle mentioned this pull request Apr 3, 2022
13 tasks
@mweinelt
Copy link
Member

mweinelt commented Apr 3, 2022

Updated to reflect reviewer feedback. PTAL.

@Mic92 Mic92 merged commit 6804184 into NixOS:master Apr 4, 2022
@Mic92 Mic92 deleted the docs branch April 4, 2022 01:11
@Artturin
Copy link
Member

Artturin commented Apr 5, 2022

The fixup commit wasn't fixed up

@Artturin
Copy link
Member

Artturin commented Apr 5, 2022

#167312

@ncfavier
Copy link
Member

Simplified the instructions in #175352.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants