mir/bind
2
0
Fork 0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-08-22 01:59:26 +00:00
Code Issues Releases Wiki Activity
Page: BIND development workflow
Pages
"main" branch partial history rewrite in August 2022 A few thoughts on Rust in BIND BIND 9 Achievements BIND 9 F2F Meeting in Warsaw, October 2019 BIND 9 Memory Explained BIND 9 PKCS11 BIND 9 Packaging BIND 9 Style Guide BIND 9.11 ESV Soft Code Freeze BIND 9.15 Plan BIND 9.17 Plan BIND 9.19 Plan BIND 9.19 Planning: CHANGES BIND 9.19 Planning: Configuration Backend BIND 9.19 Planning: DB notes BIND 9.19 Planning: Discuss new features BIND 9.19 Planning: Incident Manager Rotation BIND 9.19 Planning: Netmgr evaluation BIND 9.19 Planning: Refactor RBTDB BIND 9.19 Planning: Refactoring XFR BIND 9.19 Planning: Refactoring zone.c BIND 9.19 Planning: Refactoring BIND 9.19 Planning: Statistics System Overhaul BIND 9.19 Planning: Testing BIND 9.19 Planning: Things to do before BIND 9.18 cutoff BIND 9.21 Plan BIND Development and Release Process 2019 BIND development workflow CGroups v2 limiting resource consumption for testing CPU profiling using perf Config system requirements DNS Shotgun integration into Gitlab CI DNSSEC Key and Signing Policy (KASP) Data structure survey Debian Packages Escalation Engineer Duties Git Of Theseus Gitlab how to Integration tests wish list Internal objects and their relations Known Issues in BIND 9.18 Known Issues in BIND 9.20 Known Issues in BIND 9.21 LLM based tools thoughts Planning Policy for removing named.conf options Preparing Useful Merge Requests QP‑trie in NSD Requirements and Designs Server side Git hooks Serving stale data Stable Release Update Statistics Requirements SystemTap on 4326 remove locking from copy_namehook_lists SystemTap on main Team Meetings Testing by configuration options Updating resolver algorithm, or "who knows best, the parent or the child?" User space Probing in BIND 9 _sidebar benchmarks best practices caches dashboard home october2018meeting pytest howto stdatomic usage in master
Clone
15
BIND development workflow
Petr Špaček edited this page 2024-11-15 14:56:57 +00:00
Table of Contents

BIND development workflow

Notes from the group meeting during IETF 101:

Commits

See Linux Kernel guidance for commit messages. Most of it applies, especially the desire do describe why we are doing things. What has been done is usually visible in the code.

Describe your changes in imperative mood, e.g. “Make xyzzy do frotz” instead of “[This patch] makes xyzzy do frotz” or “[I] changed xyzzy to do frotz”, as if you are giving orders to the codebase to change its behaviour :-)

When in doubt, look at git log.

Creating a merge request

Generally, issues are used for discussion of problems and merge requests are used for discussion of the specific code used to fix the problems.

While it is possible to create a merge request and a git branch from the issue page, this isn't recommended. It clutters the MR list with merge requests that have no work in them yet, and also triggers an unnecessary pipeline run. Instead, when working on a Gitlab issue, create a development branch in your local working repository. If you give the branch a name beginning with the issue number followed by a hyphen, then the branch will automatically be associated with that issue when pushed. When ready, push the branch to isc-projects/bind9, then create a merge request to go with the branch. One way to do this is to go to the pipelines page, click on the branch name, and then click "Create merge request". Edit the commit message as necessary, and check "Remove source branch when merged".

For minor changes, it isn't always necessary to create an issue in Gitlab; just create and push a branch, then create a merge request without linking to an issue.

How to name a branch
  • If the branch is a development/PoC branch not related to any issue, then it should be login/shortname.
  • If the branch has to be attached to an issue, then it should be issuenumber/shortname. As mentioned above, this will automatically associate the branch with the issue issuenumber.
Review labels

Several review-related labels have been added to Gitlab merge requests:

  1. ~Review: Set by the author when the branch is ready to be reviewed.
  2. ~"Needs Cleanup": Can be set by either the author or the reviewer; this indicates that regardless of the current state of the code, the branch still needs to be cleaned up -- for example, by squashing commits in git rebase -i.
  3. ~"Author Merge": The author wishes to merge this branch personally and requests that no one else click the merge button, regardless of whether it's deemed ready.
Version labels

There are two sets of labels currently used for issues and merge requests:

  • blue Affects v9.x labels (e.g. ~"Affects v9.19") indicate which branches a given issues affects,
  • green v9.x labels (e.g. ~"v9.19") indicate which branches a given issue will be fixed in.

💡 NOTE: This section uses specific labels below purely for nicer formatting. So, ~"Affects v9.19" means "any blue Affects v9.x label" and ~"v9.19" means "any green v9.x label".

~"Affects v9.19" labels are only meant to be set for issues.

~"v9.19" labels are meant to be used for both issues and merge requests:

  • if a given merge request is supposed to be backported to other branches, the set of ~"v9.19" labels indicates which branches it is supposed to be merged and backported to,
  • if a given merge request is a backport, it has either the ~Backport or ~"Backport::Partial" label set and just one ~"v9.19" label indicating which branch it targets.

The above rules assume that a merge request associated with an issue fixes a problem that affects the same set of branches as the issue it is associated with. However, since a merge request does not need to be associated with a GitLab issue, one might wonder whether the ~"Affects v9.19" labels should be set for merge requests that are not associated with an issue. The answer is no. However, please note that merge requests not associated with any issue should be simple in nature. Anything non-trivial warrants creating a GitLab issue with a description of the problem being solved and marking it with the appropriate ~"Affects v9.19" labels.

The following table summarizes how labels are expected to be set:

Set this label for this entity? Issue Merge request Backport
~"Affects v9.19" ✔️ ❌ ❌
~"v9.19" ✔️ ✔️1 ✔️2

Danger/Hazard (the danger GitLab CI job run as part of every merge request pipeline) should provide helpful tips in this area in case of doubt. (dangerfile.py contains comments outlining the rules it currently follows.)

Code review

A merge request has an Assignee field and a Reviewer field. If you are assigned to one of those it will show up in "Your Merge Requests".

When you create a merge request, set the Assignee to yourself. Typically you are the one that is responsible for merging it eventually. You can change the Assignee if someone is taking over the work, but in most cases the one who created the merge request will stay the assignee.

When the merge request is ready for review, add the Review label and set the Assignee field to Unassigned. Merge requests with those attributes are ready for review.

The Reviewer field is left Unassigned. The sole purpose of this field is to request a review from a specific person. As author (or on behalf of the author) of the merge request you can set it to indicate so.

If you start reviewing a merge request, set the Assignee field to yourself. You are now the reviewer. When you are done with the review, you should assign the merge request back to the author, indicating the action item is back to the author. If you were requested to do the review in the Reviewer, make sure it is set to Unassigned when you are done reviewing the Merge request.

Changelog and release notes (since ~"v9.20")

Changelog is generated from the MR title and description - please pay extra attention to the contents of those. If the target audience of the change (from the MR title) are users or packagers, then a release note is also generated from the description.

MR title

[BRANCH][CVE-20XX-YYYY] ACTION: AUDIENCE: short description of change

  • [BRANCH]: optional, intended for backports

  • [CVE-20XX-YYYY]: optional, for issues fixing security issues with CVE

  • ACTION: required, classifies the change

    ACTION Section
    sec Security Fixes
    new New Features
    rem Removed Features
    chg Feature Changes
    fix Bug Fixes

  • AUDIENCE: required, determines if a changelog entry and/or release note should be generated from the MR description (or not)

    AUDIENCE Intended for Release note Changelog
    usr users ✅ ✅
    pkg packagers ✅ ✅
    dev developers ❌ ✅
    test testers ❌ ❌
    doc docs/support ❌ ❌
    ci infra ❌ ❌
    nil - ❌ ❌
Examples
MR title Release note Changelog Generated Text
new: usr: Add RESOLVER.ARPA to the built in empty zones ✅ ✅ Add RESOLVER.ARPA to the built in empty zones.
[9.18] chg: usr: Expose the TCP client count in statistics channel ✅ ✅ Expose the TCP client count in statistics channel.
[CVE-2023-50387] sec: usr: Fix KeyTrap ✅ ✅ [CVE-2023-50387] Fix KeyTrap.
chg: pkg: Require jemalloc >= 4.0.0 ✅ ✅ Require jemalloc >= 4.0.0.
fix: dev: Remove infinite loop on ISC_R_NOFILE ❌ ✅ Remove infinite loop on ISC_R_NOFILE.
rem: dev: Remove legacy system test runner ❌ ✅ Remove legacy system test runner.
[9.18] chg: dev: Adjust UDP zone maintenance timeouts ❌ ✅ Adjust UDP zone maintenance timeouts.
chg: nil: replace qpzone node attributes with atomics ❌ ❌
fix: nil: Fix typing mistakes in trace macros ❌ ❌
rem: test: Remove Windows builds from the CI ❌ ❌
chg: doc: Update RFC references in documentation ❌ ❌

MR description

  • use the same description for backports (+ link to original MR using Backport of MR: !XXXX)
  • link to issue with Closes: #XXXX / Fixes: #XXXX if available
  • use newlines/paragraphs when linking multiple issues: 
    Closes #XXXX
Closes #YYYY
  • pay extra attention to the description, especially if a release note or a changelog entry is to be generated - when creating/updating the MR, during review and before pressing the merge button

  1. apply a set of v9.x labels to indicate backporting intentions ↩︎

  2. apply a single v9.x label to indicate the branch it targets ↩︎