Group: Guix/PatchReviewSessions2024
(Guix systems to know) |
(Creating a worktree and initialising StackedGit) |
||
Line 121: | Line 121: | ||
== Patch review process - CLI tools == | == Patch review process - CLI tools == | ||
+ | |||
+ | |||
+ | ==== 1. Get into a constructive state of mind ==== | ||
+ | |||
+ | ==== 2. Choose something to review ==== | ||
+ | |||
+ | ==== 3. Preflight checks ==== | ||
+ | |||
+ | * [ ] check whether [https://qa.guix.gnu.org/ Guix QA] processed it (for QA go to qa.guix.gnu.org/issues/NNNN) | ||
+ | * [ ] existing package - check dependencies and reverse-dependencies | ||
+ | * [ ] existing package - check ci/git for existing update: https://git.savannah.gnu.org/cgit/guix.git/log/ | ||
+ | * [ ] check [https://issues.guix.gnu.org/ Guix Issues] for existing update: | ||
+ | * [ ] look in whereiseveryone [https://toys.whereis.みんな/ toys service] in case someone else did a a package | ||
+ | |||
+ | |||
+ | ==== 4. Set yourself as the reviewer ==== | ||
+ | Check that no-one else has set themselves as the owner by looking at the bug: | ||
+ | |||
+ | export DEBEMAIL=your-email@domain.com | ||
+ | bts --bts-server https://debbugs.gnu.org/ --mbox show NNNN | ||
+ | |||
+ | Make yourself the owner: | ||
+ | |||
+ | export DEBEMAIL=your-email@domain.com | ||
+ | bts --bts-server https://debbugs.gnu.org --mutt owner NNNN ! | ||
+ | |||
+ | |||
+ | Set a user-tag: :: | ||
+ | |||
+ | bts --bts-server https://debbugs.gnu.org --mutt user guix@debbugs.gnu.org , usertag NNNN + under-review | ||
+ | |||
+ | A bit annoyingly you will have to edit the 'user' part of the email to be 'user guix' without the rest of the email. | ||
+ | |||
+ | If you don't want to use Mutt or bts then you can also send a plain text email: | ||
+ | |||
+ | to: control@debbugs.gnu.org | ||
+ | subject: <doesn't matter> | ||
+ | user guix | ||
+ | usertag NNN under-review | ||
+ | quit | ||
+ | |||
+ | To check that your bug is now tagged correctly see this [https://debbugs.gnu.org/cgi-bin/pkgreport.cgi?tag=under-review;users=guix Debbugs under-review report]. The alternative is: | ||
+ | |||
+ | export BROWSER=w3m | ||
+ | bts --bts-server=https://debbugs.gnu.org bugs usertag:under-review users=guix status=open | ||
+ | |||
+ | ==== Create a new worktree ==== | ||
+ | Use a worktree or a branch to use with this patch and review. This is just one way to do it: | ||
+ | |||
+ | # update the local repository | ||
+ | cd workspace/guix-packages/guix | ||
+ | git checkout master | ||
+ | git pull | ||
+ | |||
+ | # create the worktree: <location it will be> -b <branch> <origin to track> --track | ||
+ | git worktree add $HOME/user/guix-packages/worktrees/issue-NNNN -b issue-NNNN origin/master --track | ||
+ | |||
+ | ===== (optional) Initialise StackedGit ===== | ||
+ | Initialise StackedGit on the worktree - I find this easiest because it shows a 'stack of patches'. | ||
+ | |||
+ | cd $HOME/user/workspace/guix-packages/worktrees/issue-NNNN | ||
+ | stg init | ||
+ | stg branch --describe "Patch review for NNNN" | ||
+ | |||
+ | ==== Build guix ==== | ||
+ | Start a separate build split to build guix: | ||
+ | |||
+ | $ guix shell --container --nesting --development guix | ||
+ | [env]$ make clean && make clean-go | ||
+ | |||
+ | [env]$ ./bootstrap | ||
+ | [env]$ ./configure --localstatedir=/var --sysconfdir=/etc | ||
+ | [env]$ make --jobs 8 | ||
+ | [env] exit | ||
+ | |||
+ | ==== Find the patch to apply ==== | ||
+ | |||
+ | |||
Revision as of 16:56, 6 March 2024
Contents
Patch Review Sessions 2024
We're running online patch review sessions so that everyone can become a patch reviewer. The goal of reviewing patches is to help Guix project accept contributions while maintaining our quality standards. Patch reviewing is a great way to learn about Guix packages and the Guix system generally.
These are great sessions if you'd like to:
- Learn how to do patch reviews together
- Do some patch reviews in a friendly hacking session
For each session please join through the Meetup link next to each event to get updates. If you don't want to join through Meetup the session is run on Jitsi at https://meet.jit.si/london-guix-meetup
Calendar
These are the sessions we're planning on running - click Expand to check the topic, and register using the Meetup link next to each one:
Our first patch review session. TBC will give an introduction to their process for reviewing patches. We'll then pick some patches to review and work through them as a group.
TBC will give an introduction to their process for reviewing patches. We'll then pick some patches to review and work through them as a group.
TBC will give an introduction to their process for reviewing patches. We'll then pick some patches to review and work through them as a group.
With an introduction by TBC. We'll look at xxxx areas.
TBC will give an introduction to their process for reviewing patches. We'll then pick some patches to review and work through them as a group.
TBC will give an introduction to their process for reviewing patches. We'll then pick some patches to review and work through them as a group.
TBC will give an introduction to their process for reviewing patches. We'll then pick some patches to review and work through them as a group.
Session Agenda
Each session is run at 18:00 UTC, 18:00 BST (London), 19:00 CET (Paris), 13:00 EST (New York) and we'll generally try to:
- Introduction to someone's patch review process (30 mins)
- Patch review together (1 hour)
This will be an informal, friendly and interactive environment where anyone can ask questions, show how they do something or make suggestions. It's on-line which restricts things a little, but the goal is to be interactive and learn together.
If there are particular topics or questions you'd like to explore please add them below.
Next session questions / topics
- [a patch you want to review / a question you have? Add it here with your name]
Guix systems
- Guix QA Automatically builds the latest patches that are sent to the devel mailing list. Take the link to patches to see what's recently been built. Click into the issue number and it will cross-link you to other places like Issues and the Patchwork instance.
- Guix Issues provides a nice front-end to the data that's stored in the Debbugs instance. It has search and an easy way to get the Message-ID.
- Debbugs guix-patches is a shared GNU project debbugs. It's old and difficult to work with - can be better if you use the Debian bts package.
- Get patches from guix-patches public inbox, or through Guix Patchwork, or through the bug number on Issues/Debbugs.
Patch review overview
The purpose of a patch review is to make it easier for committer to accept the contribution. This means checking that it follows the generally accepted standards of the project and working with the contributor to help them if possible. Not everyone is a seasoned Guix developer, so think of reviews as having a couple of levels:
- 1. Submission review
- Generally checking that the submission is in good shape:
- Patch commit message and format: correctness, no spelling or punctuation errors
- Package license: check that new package licenses are correct
- Patch application - does it apply cleanly?
- Run guix lint as this gives lots of information
- 2. Functionality review
- Check that the patch does what it's supposed to do.
- Does it build in a repeatable way?
- Does it install correctly?
- Does the package work?
- Does guix lint pass?
- 3. Code review
- Review the code for problems
- Is the code formatted in a way that's similar to other code around it
- Does the change make sense?
- Does it use guix functions correctly?
General patch review comments
- Remember someone has put a lot of effort into their contribution - approach it positively
- Guix practises one change per patch
- If there is a problem be specific about how to reproduce it
- When leaving comments for the next reviewer / committer provide some reasonable detail on what you looked at and any questions you had
- It's fine not to know something - leave a comment for the committer so they can respond
- Participate on the bug - so that your review is part of the conversation around the contribution
Patch review process - CLI tools
1. Get into a constructive state of mind
2. Choose something to review
3. Preflight checks
- [ ] check whether Guix QA processed it (for QA go to qa.guix.gnu.org/issues/NNNN)
- [ ] existing package - check dependencies and reverse-dependencies
- [ ] existing package - check ci/git for existing update: https://git.savannah.gnu.org/cgit/guix.git/log/
- [ ] check Guix Issues for existing update:
- [ ] look in whereiseveryone toys service in case someone else did a a package
4. Set yourself as the reviewer
Check that no-one else has set themselves as the owner by looking at the bug:
export DEBEMAIL=your-email@domain.com bts --bts-server https://debbugs.gnu.org/ --mbox show NNNN
Make yourself the owner:
export DEBEMAIL=your-email@domain.com bts --bts-server https://debbugs.gnu.org --mutt owner NNNN !
Set a user-tag: ::
bts --bts-server https://debbugs.gnu.org --mutt user guix@debbugs.gnu.org , usertag NNNN + under-review
A bit annoyingly you will have to edit the 'user' part of the email to be 'user guix' without the rest of the email.
If you don't want to use Mutt or bts then you can also send a plain text email:
to: control@debbugs.gnu.org subject: <doesn't matter> user guix usertag NNN under-review quit
To check that your bug is now tagged correctly see this Debbugs under-review report. The alternative is:
export BROWSER=w3m bts --bts-server=https://debbugs.gnu.org bugs usertag:under-review users=guix status=open
Create a new worktree
Use a worktree or a branch to use with this patch and review. This is just one way to do it:
# update the local repository cd workspace/guix-packages/guix git checkout master git pull
# create the worktree: <location it will be> -b <branch> <origin to track> --track git worktree add $HOME/user/guix-packages/worktrees/issue-NNNN -b issue-NNNN origin/master --track
(optional) Initialise StackedGit
Initialise StackedGit on the worktree - I find this easiest because it shows a 'stack of patches'.
cd $HOME/user/workspace/guix-packages/worktrees/issue-NNNN stg init stg branch --describe "Patch review for NNNN"
Build guix
Start a separate build split to build guix:
$ guix shell --container --nesting --development guix [env]$ make clean && make clean-go [env]$ ./bootstrap [env]$ ./configure --localstatedir=/var --sysconfdir=/etc [env]$ make --jobs 8 [env] exit
Find the patch to apply
Patch review process - Guix Emacs package
[link to the guix emacs package]
Patch review resources
-
A new Quality Assurance tool for Guix
- An overview of the Guix QA system and the work that's been done on it.
-
Guix Manual: Submitting Patches
- How to submit patches to Guix.