-
Notifications
You must be signed in to change notification settings - Fork 178
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
[Docs] Crypto signatures manuals and version script #1788
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @woju)
Documentation/devel/maintainer-manual.rst
line 20 at r1 (raw file):
https://github.com/gramineproject/gramine/issues/new?title=Release+%3Cversion%3E+TODO&body=-+%5B+%5D+draft+release+notes+%28%40%3Cowner%3E%29%0A-+%5B+%5D+draft+blogpost+%28%40%3Cowner%3E%29%0A-+%5B+%5D+draft+%23community+announcement+%28%40%3Cowner%3E%29%0A-+%5B+%5D+update+installation+instructions+%28if+a+distro+was+released+since+last+release%29+%28%40%3Cowner%3E%29%0A-+%5B+%5D+create+release+PR+%28%40%3Cowner%3E%29%0A%0Aiterate+%28update+version%2C+build+and+upload+unstable+packages%29%0A%0Afinal+stretch%3A%0A-+%5B+%5D+get+QA+signoff+%28%40%3Cowner%3E%29%0A-+%5B+%5D+approve+PR+%28%40%3Cowner%3E%29%0A-+%5B+%5D+update+version+to+final+and+push+commits+%28%40%3Cowner%3E%29%0A-+%5B+%5D+build+final+packages+%28%40%3Cowner%3E%29%0A-+%5B+%5D+upload+packages+to+release+notes+%28%40%3Cowner%3E%29%0A-+%5B+%5D+push+tag+%28%40%3Cowner%3E%29%0A-+%5B+%5D+switch+release+notes+to+pushed+tag+%28%40%3Cowner%3E%29%0A-+%5B+%5D+merge+PR+%28%40%3Cowner%3E%29%0A-+%5B+%5D+publish+release+notes+%28%40%3Cowner%3E%29%0A-+%5B+%5D+publish+blogpost+%28%40%3Cowner%3E%29%0A-+%5B+%5D+publish+on+%23community+%28%40%3Cowner%3E%29%0A create PR
Please start titles with capital letter (because you do it in other places here)
Documentation/devel/maintainer-manual.rst
line 32 at r1 (raw file):
Then set the PR on reviewable.io to be reviewed commit-by-commit. update version in PR
ditto
Documentation/devel/maintainer-manual.rst
line 37 at r1 (raw file):
.. code-block:: git reset --hard HEAD~``
What is this syntax with double-backtick?
Documentation/devel/maintainer-manual.rst
line 38 at r1 (raw file):
git reset --hard HEAD~`` scripts/release.sh X.Y~rcN``
ditto
Documentation/devel/maintainer-manual.rst
line 41 at r1 (raw file):
git push --force tag
ditto
Documentation/devel/maintainer-manual.rst
line 65 at r1 (raw file):
“Code signing” refers to the process of cryptographically siging your contributions (commits and tags), so other people are able to mathematically prove that the contribution came from the holder of particular cryptographic
holder of a particular
(add the a
)
Documentation/devel/maintainer-manual.rst
line 71 at r1 (raw file):
Generating key ^^^^^^^^^^^^^^ First, you need to generate you key using :program:`gpg`. The key need to be "sign
key need
-> key needs
Documentation/devel/maintainer-manual.rst
line 73 at r1 (raw file):
First, you need to generate you key using :program:`gpg`. The key need to be "sign only"! Otherwise, if you also add encrypt capability, people will add your key to their MUAs and will encrypt e-mail messages to you using code signing key.
What are MUAs? Please decipher. Mail User Agent?
scripts/release.sh
line 78 at r1 (raw file):
"Bump "*) commit "$V" --amend ;; *) commit "$V" ;; esac
I don't understand, why do you need this case
? Why not just bump $V; commit $V
?
e292f31
to
06a6df1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 2 of 4 files reviewed, 9 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @dimakuv)
Documentation/devel/maintainer-manual.rst
line 20 at r1 (raw file):
Previously, dimakuv (Dmitrii Kuvaiskii) wrote…
Please start titles with capital letter (because you do it in other places here)
Done.
Documentation/devel/maintainer-manual.rst
line 32 at r1 (raw file):
Previously, dimakuv (Dmitrii Kuvaiskii) wrote…
ditto
Done.
Documentation/devel/maintainer-manual.rst
line 37 at r1 (raw file):
Previously, dimakuv (Dmitrii Kuvaiskii) wrote…
What is this syntax with double-backtick?
It's a leftover. Removed.
Documentation/devel/maintainer-manual.rst
line 38 at r1 (raw file):
Previously, dimakuv (Dmitrii Kuvaiskii) wrote…
ditto
Done.
Documentation/devel/maintainer-manual.rst
line 41 at r1 (raw file):
Previously, dimakuv (Dmitrii Kuvaiskii) wrote…
ditto
Done.
Documentation/devel/maintainer-manual.rst
line 65 at r1 (raw file):
Previously, dimakuv (Dmitrii Kuvaiskii) wrote…
holder of a particular
(add thea
)
Done.
Documentation/devel/maintainer-manual.rst
line 71 at r1 (raw file):
Previously, dimakuv (Dmitrii Kuvaiskii) wrote…
key need
->key needs
Done.
Documentation/devel/maintainer-manual.rst
line 73 at r1 (raw file):
Previously, dimakuv (Dmitrii Kuvaiskii) wrote…
What are MUAs? Please decipher. Mail User Agent?
Done.
scripts/release.sh
line 78 at r1 (raw file):
Previously, dimakuv (Dmitrii Kuvaiskii) wrote…
I don't understand, why do you need this
case
? Why not justbump $V; commit $V
?
For release we're creating two commits: the first one changes to version from X.{Y-1}
to X.Y~rcZ
, and the second one changes to X.Ypost~UNRELEASED
. So if you're changing the commit stack from ~rcZ
to ~rc{Z+1}
(or to final X.Y
, doesn't matter for this argument), there are two options:
- roll back two commits and recreate both of them (
git reset HEAD~2 && sed && git commit && sed && git commit
); - roll back one commit, amend the first one and recreate the second one (
git reset HEAD~ && sed && git commit --amend && sed && git commit
).
The problem is, if you're updating from ~rcZ
to ~rc{Z+1}
, the first commit also probably contains non-trivial changes to debian/changelog
, which can't be recreated, so --amend
is the obvious solution, and that's what manual and the comment suggests. But if you're doing this commit stack for the first time, you need to create both commits anyway and you're not rolling back anything. So, you need to have both code paths available.
The difference is between them is detected based on title of the commit on the HEAD of the branch. If it starts with Bump
, then we've most likely just reset the second commit and we're in the middle of the stack, so we just --amend
. If not Bump
, then we're creating the stack for the first time.
Do you want to have all of this in a comment?
I've fixed whitespace. Previously it said "Bump Gramine "*
, but I removed Gramine
to make it applicable to all repos (not just gramineproject/gramine
, but also gramineproject/contrib
etc.), but failed to adjust spaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed all commit messages.
Reviewable status: 2 of 4 files reviewed, 13 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @dimakuv and @woju)
a discussion (no related file):
Can we wait with this until #1784 is merged? So I could test it on the new Sphinx before merging.
Documentation/devel/maintainer-manual.rst
line 70 at r2 (raw file):
Generating key ^^^^^^^^^^^^^^
Please add en empty line here.
Documentation/devel/DCO/index.rst
line 6 at r2 (raw file):
For cryptographical “code signing”, as opposed to “signing off” your commits, please refer to :doc:`../maintainer-manual`.
Could you link to the specific section there?
scripts/release.sh
line 3 at r2 (raw file):
#!/bin/sh # SPDX-License-Identifier: LGPL-3.0-or-later # SPDX-FileCopyrightText: 2024 Wojtek Porczyk <woju@invisiblethingslab.com>
We don't use this format in other files. Also, the copyright should be Intel?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @woju)
scripts/release.sh
line 78 at r1 (raw file):
Previously, woju (Wojtek Porczyk) wrote…
For release we're creating two commits: the first one changes to version from
X.{Y-1}
toX.Y~rcZ
, and the second one changes toX.Ypost~UNRELEASED
. So if you're changing the commit stack from~rcZ
to~rc{Z+1}
(or to finalX.Y
, doesn't matter for this argument), there are two options:
- roll back two commits and recreate both of them (
git reset HEAD~2 && sed && git commit && sed && git commit
);- roll back one commit, amend the first one and recreate the second one (
git reset HEAD~ && sed && git commit --amend && sed && git commit
).The problem is, if you're updating from
~rcZ
to~rc{Z+1}
, the first commit also probably contains non-trivial changes todebian/changelog
, which can't be recreated, so--amend
is the obvious solution, and that's what manual and the comment suggests. But if you're doing this commit stack for the first time, you need to create both commits anyway and you're not rolling back anything. So, you need to have both code paths available.The difference is between them is detected based on title of the commit on the HEAD of the branch. If it starts with
Bump
, then we've most likely just reset the second commit and we're in the middle of the stack, so we just--amend
. If notBump
, then we're creating the stack for the first time.Do you want to have all of this in a comment?
I've fixed whitespace. Previously it said
"Bump Gramine "*
, but I removedGramine
to make it applicable to all repos (not justgramineproject/gramine
, but alsogramineproject/contrib
etc.), but failed to adjust spaces.
I'm fine with this explanation here. For historical digging, if ever needed, we'll get to your comment on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 9 files reviewed, 4 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @dimakuv and @mkow)
a discussion (no related file):
Previously, mkow (Michał Kowalczyk) wrote…
Can we wait with this until #1784 is merged? So I could test it on the new Sphinx before merging.
It's merged. Do you want a rebase?
Documentation/devel/maintainer-manual.rst
line 70 at r2 (raw file):
Previously, mkow (Michał Kowalczyk) wrote…
Please add en empty line here.
Done.
Documentation/devel/DCO/index.rst
line 6 at r2 (raw file):
Previously, mkow (Michał Kowalczyk) wrote…
Could you link to the specific section there?
Done.
scripts/release.sh
line 3 at r2 (raw file):
Previously, mkow (Michał Kowalczyk) wrote…
We don't use this format in other files. Also, the copyright should be Intel?
IDK about header, I found it somewhere, maybe we should use it project wide? For now I've reverted to the version without header for consistency.
About copyright, I think this script might have been done on scaffolding time this year. Probably not Intel.
907b069
to
7057718
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 10 files reviewed, 5 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @dimakuv and @mkow)
Documentation/devel/maintainer-manual.rst
line 9 at r3 (raw file):
Create new checklist issue (fill all ``<variable>`` before submitting): .. new-issue:: Release <version> checklist
I'm sorry for this custom directive, but I got fed up with manual editing of the query string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 10 files reviewed, 5 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @dimakuv and @woju)
a discussion (no related file):
Previously, woju (Wojtek Porczyk) wrote…
It's merged. Do you want a rebase?
Yes, please.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 10 files reviewed, 5 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @dimakuv and @mkow)
a discussion (no related file):
Previously, mkow (Michał Kowalczyk) wrote…
Yes, please.
It already is rebased. I think I rebased it when writing this second commit about verifying signatures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 8 of 10 files at r3, all commit messages.
Reviewable status: 8 of 10 files reviewed, 5 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @mkow)
a discussion (no related file):
Previously, woju (Wojtek Porczyk) wrote…
It already is rebased. I think I rebased it when writing this second commit about verifying signatures.
I enabled this PR in ReadTheDocs: https://gramine.readthedocs.io/en/woju-docs-202402/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 10 files at r3.
Reviewable status: all files reviewed, 12 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @mkow and @woju)
Documentation/verify-sig.rst
line 115 at r3 (raw file):
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ If you happen to have your own PGP key pair, you can choose sign the key with
choose sign
-> choose to sign
Documentation/verify-sig.rst
line 178 at r3 (raw file):
.. no, I don't have "0000000000000000" key If you know what you're doing, you can use another singing command in place of
singing
-> signing
Documentation/verify-sig.rst
line 206 at r3 (raw file):
gpg: using EDDSA key 9C4D27D9157EF771A4283926044D9664E7A77E16 gpg: Good signature from "Wojciech Porczyk (Gramine code signing key) <woju@invisiblethingslab.com>" [full] % git verify-tag v1.6.2
Could you add an empty line before this command?
Documentation/verify-sig.rst
line 254 at r3 (raw file):
[...] % % git verify-commit a971e30f3430b4b8079ec42f5d035ced68130bdc
% %
-> %
Documentation/devel/maintainer-manual.rst
line 80 at r3 (raw file):
signature. For details, please read :doc:`DCO/index`. “Code signing” refers to the process of cryptographically siging your
siging
-> signing
Documentation/devel/maintainer-manual.rst
line 89 at r3 (raw file):
^^^^^^^^^^^^^^ First, you need to generate you key using :program:`gpg`. The key needs to be
you key
-> your key
Documentation/devel/maintainer-manual.rst
line 94 at r3 (raw file):
to you using code signing key. This is not desired, the key generated for the purpose of code signing should not be used in any other context (e.g. e-mail or singing code in other projects).
singing
-> signing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 8 of 10 files reviewed, 12 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @dimakuv and @mkow)
a discussion (no related file):
Previously, dimakuv (Dmitrii Kuvaiskii) wrote…
I enabled this PR in ReadTheDocs: https://gramine.readthedocs.io/en/woju-docs-202402/
Adding separate versions is not needed anymore, since we build all PRs: https://gramine--1788.org.readthedocs.build/en/1788/ (you can substitute any valid number of a PR).
Webhook works, just reporting the status doesn't.
Documentation/verify-sig.rst
line 115 at r3 (raw file):
Previously, dimakuv (Dmitrii Kuvaiskii) wrote…
choose sign
->choose to sign
Done.
Documentation/verify-sig.rst
line 178 at r3 (raw file):
Previously, dimakuv (Dmitrii Kuvaiskii) wrote…
singing
->signing
Done.
Documentation/verify-sig.rst
line 206 at r3 (raw file):
Previously, dimakuv (Dmitrii Kuvaiskii) wrote…
Could you add an empty line before this command?
Just empty line, no, I don't want to, because it is (was) a single listing. I guess I can split it into two paragraphs instead. Did the same to verifying commits too.
Documentation/verify-sig.rst
line 254 at r3 (raw file):
Previously, dimakuv (Dmitrii Kuvaiskii) wrote…
% %
->%
Done.
Documentation/devel/maintainer-manual.rst
line 80 at r3 (raw file):
Previously, dimakuv (Dmitrii Kuvaiskii) wrote…
siging
->signing
Done.
Documentation/devel/maintainer-manual.rst
line 89 at r3 (raw file):
Previously, dimakuv (Dmitrii Kuvaiskii) wrote…
you key
->your key
Done.
Documentation/devel/maintainer-manual.rst
line 94 at r3 (raw file):
Previously, dimakuv (Dmitrii Kuvaiskii) wrote…
singing
->signing
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @mkow)
a discussion (no related file):
Previously, woju (Wojtek Porczyk) wrote…
Adding separate versions is not needed anymore, since we build all PRs: https://gramine--1788.org.readthedocs.build/en/1788/ (you can substitute any valid number of a PR).
Webhook works, just reporting the status doesn't.
Ah, awesome.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 8 of 10 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @woju)
-- commits
line 7 at r4:
TODO for myself: verify commit split before the rebase.
Documentation/index.rst
line 218 at r4 (raw file):
devel/onboarding devel/setup devel/maintainer-manual
I'd push this down on the list, it's important only to very few people and will be read much less often than the other entries here.
Documentation/verify-sig.rst
line 58 at r4 (raw file):
Then check the key fingerprint. If you're sure the key is correct, you need to mark it as trusted.
Suggestion:
Then check the key fingerprint. After ensuring the key is correct, you can
mark it as trusted.
Documentation/devel/maintainer-manual.rst
line 1 at r4 (raw file):
Maintainer's manual
I think this is rather a "Release maintainer's manual"?
Documentation/devel/maintainer-manual.rst
line 35 at r4 (raw file):
- [ ] publish on #community (@<owner>) Create PR
Suggestion:
Create a PR
Documentation/devel/maintainer-manual.rst
line 47 at r4 (raw file):
Then set the PR on reviewable.io to be reviewed commit-by-commit. Update version in PR
Suggestion:
Update version in the PR
Documentation/devel/maintainer-manual.rst
line 74 at r4 (raw file):
The meaning of it depends on the project, quoting git docs:
The meaning of a signoff depends on the project to which you’re committing.
I'd also change the last sentence here accordingly.
Suggestion:
“Signing off” is a |~| legal device for a |~| sort of signature by which you
usually assert that you are holding copyrights to the code you're submitting (or
Documentation/devel/maintainer-manual.rst
line 91 at r4 (raw file):
First, you need to generate your own key pair using :program:`gpg`. The key needs to be "sign only"! Otherwise, if you also add encrypt capability, people will add your key to their :abbr:`MUA (Mail User Agent)`\ s and will encrypt
I think this term is much more popular nowadays and thus doesn't need an explanation.
Suggestion:
email clients
Documentation/devel/maintainer-manual.rst
line 192 at r4 (raw file):
git config --global gpg.program qubes-gpg-client-wrapper and remember to set ``QUBES_GPG_DOMAIN`` envrionment variable in your shell
Suggestion:
environment
scripts/release.sh
line 3 at r2 (raw file):
Previously, woju (Wojtek Porczyk) wrote…
IDK about header, I found it somewhere, maybe we should use it project wide? For now I've reverted to the version without header for consistency.
About copyright, I think this script might have been done on scaffolding time this year. Probably not Intel.
Then it should be ITL?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 7 of 10 files reviewed, 10 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @dimakuv, @mkow, and @woju)
Documentation/index.rst
line 218 at r4 (raw file):
Previously, mkow (Michał Kowalczyk) wrote…
I'd push this down on the list, it's important only to very few people and will be read much less often than the other entries here.
Hmm, right, it certainly should be below coding-style
. I moved it to the bottom of technical ones, leaving only legal stuff below.
Documentation/verify-sig.rst
line 58 at r4 (raw file):
Then check the key fingerprint. If you're sure the key is correct, you need to mark it as trusted.
Done.
Documentation/devel/maintainer-manual.rst
line 1 at r4 (raw file):
Previously, mkow (Michał Kowalczyk) wrote…
I think this is rather a "Release maintainer's manual"?
No, it also describes code signing which I hope will be used by all maintainers (maybe even all contributors), not just me. Also we don't have a position of "release maintainer" or "release manager", all maintainers can (eventually should be able to) do a release (i.e. sign a tag).
If you want it to be release manual, I can move the section about generating keys and configuring git to devel/setup.rst
(right now it's only about configuring vim).
Documentation/devel/maintainer-manual.rst
line 35 at r4 (raw file):
- [ ] publish on #community (@<owner>) Create PR
Done.
Documentation/devel/maintainer-manual.rst
line 47 at r4 (raw file):
Then set the PR on reviewable.io to be reviewed commit-by-commit. Update version in PR
Done.
(I also rewrote next heading to "Create a tag" to match all previous headings).
Documentation/devel/maintainer-manual.rst
line 74 at r4 (raw file):
Previously, mkow (Michał Kowalczyk) wrote…
The meaning of it depends on the project, quoting git docs:
The meaning of a signoff depends on the project to which you’re committing.
I'd also change the last sentence here accordingly.
I think it should be as written, I describe what signing off means in our project. I can add "(in our project)" in some strategic place, or add a sentence in parens like >(In other projects "singing off" might have slightly different meaning, check the details with the respective project every time.)
Documentation/devel/maintainer-manual.rst
line 91 at r4 (raw file):
Previously, mkow (Michał Kowalczyk) wrote…
I think this term is much more popular nowadays and thus doesn't need an explanation.
There are different "email clients" doing different stuff, so I wanted to optimise the phraseology for precison. This is manual for maintainers, there's no need to dumb it down.
Documentation/devel/maintainer-manual.rst
line 192 at r4 (raw file):
git config --global gpg.program qubes-gpg-client-wrapper and remember to set ``QUBES_GPG_DOMAIN`` envrionment variable in your shell
Done.
scripts/release.sh
line 3 at r2 (raw file):
Previously, mkow (Michał Kowalczyk) wrote…
Then it should be ITL?
No, in ITL we don't reassign copyrights (see Qubes repos).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 3 of 3 files at r5, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @woju)
Documentation/devel/maintainer-manual.rst
line 1 at r4 (raw file):
No, it also describes code signing which I hope will be used by all maintainers
I agree with that, but this document is more specific, and the title should describe its contents well - "Maintainer's manual" sounds like a very broad document about everything relating to maintaining Gramine. And at the same time, when looking at ToC you won't easily find where exactly packaging/releasing procedures are described.
So - describe the contents, not the intended reader group.
Documentation/devel/maintainer-manual.rst
line 74 at r4 (raw file):
I describe what signing off means in our project
That's my problem with the current version, nowhere it says that it's only about how it works in our project. Currently it sounds to me like a generic explanation of this git feature: "“Signing off” is a legal device [...] by which you assert that you are holding copyrights".
I can add "(in our project)" in some strategic place, or add a sentence in parens like >(In other projects "singing off" might have slightly different meaning, check the details with the respective project every time.)
+1, both do the work IMO
Documentation/devel/maintainer-manual.rst
line 91 at r4 (raw file):
Previously, woju (Wojtek Porczyk) wrote…
There are different "email clients" doing different stuff, so I wanted to optimise the phraseology for precison. This is manual for maintainers, there's no need to dumb it down.
Maybe I'm missing something, but what's the difference between MUA and email client?
Signed-off-by: Wojtek Porczyk <woju@invisiblethingslab.com>
Signed-off-by: Wojtek Porczyk <woju@invisiblethingslab.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 9 of 10 files reviewed, 5 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @mkow)
Documentation/devel/maintainer-manual.rst
line 1 at r4 (raw file):
Previously, mkow (Michał Kowalczyk) wrote…
No, it also describes code signing which I hope will be used by all maintainers
I agree with that, but this document is more specific, and the title should describe its contents well - "Maintainer's manual" sounds like a very broad document about everything relating to maintaining Gramine. And at the same time, when looking at ToC you won't easily find where exactly packaging/releasing procedures are described.
So - describe the contents, not the intended reader group.
"Code signing and release manual"? Because those 2 topics are very different, they're connected just by the fact they're intended for maintainers, so I don't thing "Maintainer's manual" is wrong.
Or I can split those in 2 documents, 'devel/release.rstand put keygen into
devel/setup.rst`.
Documentation/devel/maintainer-manual.rst
line 74 at r4 (raw file):
Previously, mkow (Michał Kowalczyk) wrote…
I describe what signing off means in our project
That's my problem with the current version, nowhere it says that it's only about how it works in our project. Currently it sounds to me like a generic explanation of this git feature: "“Signing off” is a legal device [...] by which you assert that you are holding copyrights".
I can add "(in our project)" in some strategic place, or add a sentence in parens like >(In other projects "singing off" might have slightly different meaning, check the details with the respective project every time.)
+1, both do the work IMO
Done.
Documentation/devel/maintainer-manual.rst
line 91 at r4 (raw file):
Previously, mkow (Michał Kowalczyk) wrote…
Maybe I'm missing something, but what's the difference between MUA and email client?
There are several kinds of email software that technically are clients (in the "client-server architecture" sense): MTA (Mail Transport Agent) is also a client of SMTP protocol, but it performs different role: forwards email to "server", which may be another MTA or MDA (... Delivery Agent). There are MRAs (... Retrieval Agents), which download email from MDAs, but can't show it to people, so they're closer to MTA, just with reversed polarity (they're clients of IMAP and POP3 protocols instead of SMTP). Email packages typically package several of those roles into one more or less integrated solution (Thunderbird is MUA+MRA+limited MTA capabilities, Postfix is MSA+MTA+limited MDA, and so on). For this reason email people don't like to use unqualified "client" (or "server"), because it's immediately met with "yeah, which client?".
https://en.wikipedia.org/wiki/Email_agent_(infrastructure),
https://en.wikipedia.org/wiki/Simple_Mail_Transfer_Protocol#Mail_processing_model, and diagram in https://en.wikipedia.org/wiki/Message_delivery_agent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @dimakuv, @kailun-qin, and @woju)
Documentation/devel/maintainer-manual.rst
line 1 at r4 (raw file):
"Code signing and release manual"
Sounds good to me. We use code signing only for the release tags, so it's not a bad idea to be placed in the very same document.
Documentation/devel/maintainer-manual.rst
line 91 at r4 (raw file):
Previously, woju (Wojtek Porczyk) wrote…
There are several kinds of email software that technically are clients (in the "client-server architecture" sense): MTA (Mail Transport Agent) is also a client of SMTP protocol, but it performs different role: forwards email to "server", which may be another MTA or MDA (... Delivery Agent). There are MRAs (... Retrieval Agents), which download email from MDAs, but can't show it to people, so they're closer to MTA, just with reversed polarity (they're clients of IMAP and POP3 protocols instead of SMTP). Email packages typically package several of those roles into one more or less integrated solution (Thunderbird is MUA+MRA+limited MTA capabilities, Postfix is MSA+MTA+limited MDA, and so on). For this reason email people don't like to use unqualified "client" (or "server"), because it's immediately met with "yeah, which client?".
https://en.wikipedia.org/wiki/Email_agent_(infrastructure),
https://en.wikipedia.org/wiki/Simple_Mail_Transfer_Protocol#Mail_processing_model, and diagram in https://en.wikipedia.org/wiki/Message_delivery_agent
This differentiation seem to be super specific to email client developers. Also, even the links you pasted seem to say that "(e)mail client" == "MUA":
- https://en.wikipedia.org/wiki/Email_client:
"An email client, email reader or, more formally, message user agent (MUA)"
- https://en.wikipedia.org/wiki/Simple_Mail_Transfer_Protocol#Mail_processing_model
[...] a mail client (mail user agent, MUA)
I think "MUA" is a rather confusing and not a well-known term, while "email client" is clear and doesn't require any additional explanations to be understood.
@kailun-qin, @dimakuv: any preferences?
How to test this PR?
Will really be tested during new release...
This change is