X-Git-Url: https://git.libre-soc.org/?a=blobdiff_plain;f=docs%2Fsubmittingpatches.html;h=fb614a790b6a0debbe6cf766af3c0b9513b987f9;hb=f3c1833b775a3e0b0d1291ad768fbb4bb982ec22;hp=d8b9fc3b643d295af0675c04de38d474ae56f2a1;hpb=82dc168f51e35fe878d9d8eec79169dea82a7e34;p=mesa.git diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html index d8b9fc3b643..fb614a790b6 100644 --- a/docs/submittingpatches.html +++ b/docs/submittingpatches.html @@ -42,10 +42,8 @@ components. git bisect.)
  • Patches should be properly formatted.
  • Patches should be sufficiently tested before submitting. -
  • Patches should be submitted -to mesa-dev or with -a merge request -for review. +
  • Patches should be submitted via a merge request for +review. @@ -58,77 +56,70 @@ log uses 4 spaces of indentation (4 + 75 < 80).
  • The first line should be a short, concise summary of the change prefixed with a module name. Examples:
    -    mesa: Add support for querying GL_VERTEX_ATTRIB_ARRAY_LONG
    +mesa: Add support for querying GL_VERTEX_ATTRIB_ARRAY_LONG
     
    -    gallium: add PIPE_CAP_DEVICE_RESET_STATUS_QUERY
    +gallium: add PIPE_CAP_DEVICE_RESET_STATUS_QUERY
     
    -    i965: Fix missing type in local variable declaration.
    +i965: Fix missing type in local variable declaration.
     
  • Subsequent patch comments should describe the change in more detail, if needed. For example:
    -    i965: Remove end-of-thread SEND alignment code.
    +i965: Remove end-of-thread SEND alignment code.
     
    -    This was present in Eric's initial implementation of the compaction code
    -    for Sandybridge (commit 077d01b6). There is no documentation saying this
    -    is necessary, and removing it causes no regressions in piglit on any
    -    platform.
    +This was present in Eric's initial implementation of the compaction code
    +for Sandybridge (commit 077d01b6). There is no documentation saying this
    +is necessary, and removing it causes no regressions in piglit on any
    +platform.
     
  • A "Signed-off-by:" line is not required, but not discouraged either. -
  • If a patch addresses a bugzilla issue, that should be noted in the -patch comment. For example: +
  • If a patch addresses an issue in gitlab, use the Closes: tag +For example:
    -   Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89689
    +Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/1
     
    +

    Prefer the full url to just Closes: #1, since the url makes it +easier to get to the bug page from git log

    +Do not use the Fixes: tag for this! Mesa already uses Fixes for something else. +
  • If a patch addresses a issue introduced with earlier commit, that should be noted in the patch comment. For example:
    -   Fixes: d7b3707c612 "util/disk_cache: use stat() to check if entry is a directory"
    +Fixes: d7b3707c612 "util/disk_cache: use stat() to check if entry is a directory"
     
    -
  • You can produce those fixes lines by running
    git config --global pretty.fixes 'Fixes: %h ("%s")'
    once and then using
    git show -s --pretty=fixes <sha1>
    +
  • You can produce those fixes lines by running +
    git config --global alias.fixes "show -s --pretty='format:Fixes: %h (\"%s\")'"
    +once and then using
    git fixes <sha1>
  • If there have been several revisions to a patch during the review process, they should be noted such as in this example:
    -    st/mesa: add ARB_texture_stencil8 support (v4)
    -
    -    if we support stencil texturing, enable texture_stencil8
    -    there is no requirement to support native S8 for this,
    -    the texture can be converted to x24s8 fine.
    -
    -    v2: fold fixes from Marek in:
    -       a) put S8 last in the list
    -       b) fix renderable to always test for d/s renderable
    -        fixup the texture case to use a stencil only format
    -        for picking the format for the texture view.
    -    v3: hit fallback for getteximage
    -    v4: put s8 back in front, it shouldn't get picked now (Ilia)
    +st/mesa: add ARB_texture_stencil8 support (v4)
    +
    +if we support stencil texturing, enable texture_stencil8
    +there is no requirement to support native S8 for this,
    +the texture can be converted to x24s8 fine.
    +
    +v2: fold fixes from Marek in:
    +   a) put S8 last in the list
    +   b) fix renderable to always test for d/s renderable
    +     fixup the texture case to use a stencil only format
    +     for picking the format for the texture view.
    +v3: hit fallback for getteximage
    +v4: put s8 back in front, it shouldn't get picked now (Ilia)
     
  • If someone tested your patch, document it with a line like this:
    -    Tested-by: Joe Hacker <jhacker@foo.com>
    +Tested-by: Joe Hacker <jhacker@foo.com>
     
  • If the patch was reviewed (usually the case) or acked by someone, that should be documented with:
    -    Reviewed-by: Joe Hacker <jhacker@foo.com>
    -    Acked-by: Joe Hacker <jhacker@foo.com>
    +Reviewed-by: Joe Hacker <jhacker@foo.com>
    +Acked-by: Joe Hacker <jhacker@foo.com>
     
  • If sending later revision of a patch, add all the tags - ack, r-b, Cc: mesa-stable and/or other. This provides reviewers with quick feedback if the patch has already been reviewed. -
  • In order for your patch to reach the prospective reviewer easier/faster, -use the script scripts/get_reviewer.pl to get a list of individuals and include -them in the CC list. -

    -Please use common sense and do not blindly add everyone. -

    -
    -    $ scripts/get_reviewer.pl --help # to get the help screen
    -    $ scripts/get_reviewer.pl -f src/egl/drivers/dri2/platform_android.c
    -    Rob Herring <robh@kernel.org> (reviewer:ANDROID EGL SUPPORT,added_lines:188/700=27%,removed_lines:58/283=20%)
    -    Tomasz Figa <tfiga@chromium.org> (reviewer:ANDROID EGL SUPPORT,authored:12/41=29%,added_lines:308/700=44%,removed_lines:115/283=41%)
    -    Emil Velikov <emil.l.velikov@gmail.com> (authored:13/41=32%,removed_lines:76/283=27%)
    -
    @@ -155,7 +146,7 @@ to check for regressions.

    -As mentioned at the begining, patches should be bisectable. +As mentioned at the beginning, patches should be bisectable. A good way to test this is to make use of the `git rebase` command, to run your tests on each commit. Assuming your branch is based off origin/master, you can run: @@ -168,67 +159,13 @@ replacing "meson test" with whatever other test you want to run.

    -

    Submitting Patches

    -Patches may be submitted to the Mesa project by -email or with a -GitLab merge request. To prevent -duplicate code review, only use one method to submit your changes. -

    - -

    Mailing Patches

    - -

    -Patches may be sent to the mesa-dev mailing list for review: - -mesa-dev@lists.freedesktop.org. -When submitting a patch make sure to use -git send-email -rather than attaching patches to emails. Sending patches as -attachments prevents people from being able to provide in-line review -comments. +Patches are submitted to the Mesa project via a +GitLab Merge Request.

    -

    -When submitting follow-up patches you can use --in-reply-to to make v2, v3, -etc patches show up as replies to the originals. This usually works well -when you're sending out updates to individual patches (as opposed to -re-sending the whole series). Using --in-reply-to makes -it harder for reviewers to accidentally review old patches. -

    - -

    -When submitting follow-up patches you should also login to -patchwork and change the -state of your old patches to Superseded. -

    - -

    -Some companies' mail server automatically append a legal disclaimer, -usually containing something along the lines of "The information in this -email is confidential" and "distribution is strictly prohibited". -

    -

    -These legal notices prevent us from being able to accept your patch, -rendering the whole process pointless. Please make sure these are -disabled before sending your patches. (Note that you may need to contact -your email administrator for this.) -

    - -

    GitLab Merge Requests

    - -

    - GitLab Merge - Requests (MR) can also be used to submit patches for Mesa. -

    - -

    - If the MR may have interest for most of the Mesa community, you can - send an email to the mesa-dev email list including a link to the MR. - Don't send the patch to mesa-dev, just the MR link. -

    Add labels to your MR to help reviewers find it. For example:

    @@ -277,23 +214,22 @@ your email administrator for this.)

    Reviewing Patches

    - To participate in code review, you should monitor the - - mesa-dev email list and the GitLab - Mesa Merge - Requests page. + To participate in code review, you can monitor the GitLab Mesa + Merge + Requests page, and/or register for notifications in your gitlab + settings.

    -When you've reviewed a patch on the mailing list, please be unambiguous -about your review. That is, state either +When you've reviewed a patch, please be unambiguous about your review. + That is, state either

    -    Reviewed-by: Joe Hacker <jhacker@foo.com>
    +Reviewed-by: Joe Hacker <jhacker@foo.com>
     
    or
    -    Acked-by: Joe Hacker <jhacker@foo.com>
    +Acked-by: Joe Hacker <jhacker@foo.com>
     

    Rather than saying just "LGTM" or "Seems OK". @@ -303,7 +239,7 @@ Rather than saying just "LGTM" or "Seems OK". If small changes are suggested, it's OK to say something like:

    -   With the above fixes, Reviewed-by: Joe Hacker <jhacker@foo.com>
    +With the above fixes, Reviewed-by: Joe Hacker <jhacker@foo.com>
     

    which tells the patch author that the patch can be committed, as long @@ -320,7 +256,7 @@ When providing a Reviewed-by, Acked-by, or Tested-by tag in a gitlab MR, enclose the tag in backticks:

    -  `Reviewed-by: Joe Hacker <jhacker@example.com>`
    +`Reviewed-by: Joe Hacker <jhacker@example.com>`

    This is the markdown format for literal, and will prevent gitlab from hiding the < and > symbols. @@ -342,13 +278,14 @@ release.

    -Note: resending patch identical to one on mesa-dev@ or one that differs only -by the extra mesa-stable@ tag is not recommended. +Please DO NOT send patches to +mesa-stable@lists.freedesktop.org, it is not monitored actively and is a +historical artifact.

    If you are not the author of the original patch, please Cc: them in your @@ -367,31 +304,27 @@ you should add an appropriate note to the commit message.

    +Using a "fixes tag" as described in Patch formatting +is the preferred way to nominate a commit that you know ahead of time should be +backported. There are scripts that will figure out which releases to apply the +patch to automatically, so you don't need to figure it out. +

    + +

    +Alternatively, you may use a "CC:" tag. + Here are some examples of such a note:

    -CC: <mesa-stable@lists.freedesktop.org>
    +CC: 20.0 19.3 <mesa-stable@lists.freedesktop.org>
     
    -Simply adding the CC to the mesa-stable list address is adequate to nominate -the commit for all the active stable branches. If the commit is not applicable -for said branch the stable-release manager will reply stating so. - -This "CC" syntax for patch nomination will cause patches to automatically be -copied to the mesa-stable@ mailing list when you use "git send-email" to send -patches to the mesa-dev@ mailing list. If you prefer using --suppress-cc that -won't have any negative effect on the patch nomination. -

    -Note: by removing the tag [as the commit is pushed] the patch is -explicitly rejected from inclusion in the stable branch(es). -Thus, drop the line only if you want to cancel the nomination. +Using the CC tag should include the stable branches you want +to nominate the patch to. If you do not provide any version it is nominated to +all active stable branches.

    -Alternatively, if one uses the "Fixes" tag as described in the "Patch formatting" -section, it nominates a commit for all active stable branches that include the -commit that is referred to. -

    Criteria for accepting patches to the stable branch

    Mesa has a designated release manager for each stable branch, and the release @@ -440,9 +373,6 @@ If the patch complies with the rules it will be manager will reply to the patch in question stating why the patch has been rejected or would request a backport. -A summary of all the picked/rejected patches will be presented in the -pre-release announcement. - The stable-release manager may at times need to force-push changes to the stable branches, for example, to drop a previously-picked patch that was later identified as causing a regression). These force-pushes may cause changes to @@ -451,16 +381,22 @@ yourself warned.

    Sending backports for the stable branch

    -By default merge conflicts are resolved by the stable-release manager. In which -case he/she should provide a comment about the changes required, alongside the -Conflicts section. Summary of which will be provided in the -pre-release announcement. +By default merge conflicts are resolved by the stable-release manager. The +release maintainer should resolve trivial conflicts, but for complex conflicts +they should ask the original author to provide a backport or de-nominate the +patch.

    -Developers are interested in sending backports are recommended to use either a -[BACKPORT #branch] subject prefix or provides similar information -within the commit summary. +For patches that either need to be nominated after they've landed in master, or +that are known ahead of time to not not apply cleanly to a stable branch (such +as due to a rename), using a gitlab MR is most appropriate. + +The MR should be based on and target the staging/year.quarter branch, not on +the year.quarter branch, per the stable branch policy. + +Assigning the MR to release maintainer for said branch or mentioning them is +helpful, but not required.

    Git tips

    @@ -469,28 +405,23 @@ within the commit summary.
  • git rebase -i ... is your friend. Don't be afraid to use it.
  • Apply a fixup to commit FOO.
    -    git add ...
    -    git commit --fixup=FOO
    -    git rebase -i --autosquash ...
    +git add ...
    +git commit --fixup=FOO
    +git rebase -i --autosquash ...
     
  • Test for build breakage between patches e.g last 8 commits.
    -    git rebase -i --exec="ninja -C build/" HEAD~8
    +git rebase -i --exec="ninja -C build/" HEAD~8
     
  • Sets the default mailing address for your repo.
    -    git config --local sendemail.to mesa-dev@lists.freedesktop.org
    +git config --local sendemail.to mesa-dev@lists.freedesktop.org
     
  • Add version to subject line of patch series in this case for the last 8 commits before sending.
    -    git send-email --subject-prefix="PATCH v4" HEAD~8
    -    git send-email -v4 @~8 # shorter version, inherited from git format-patch
    -
    -
  • Configure git to use the get_reviewer.pl script interactively. Thus you -can avoid adding the world to the CC list. -
    -    git config sendemail.cccmd "./scripts/get_reviewer.pl -i"
    +git send-email --subject-prefix="PATCH v4" HEAD~8
    +git send-email -v4 @~8 # shorter version, inherited from git format-patch