Re: [gentoo-portage-dev] Signing off patches
On Sat, 18 Jan 2014 08:43:12 -0800 W. Trevor King wk...@tremily.us wrote: On Sat, Jan 18, 2014 at 04:02:02PM +0100, Tom Wijsman wrote: I think the idea is that you shouldn't need to refer to an external resource like the mailing list to understand the idea behind the patch, Either someone cares about the background of a patch or he/she doesn't. The idea is not documented by these annotations, hence if one wants to know the reasoning behind the certain way it is implemented he/she will have to go to the mailing list to know that. That is if the existing comments / commit message were insufficient for what one wonders about. or the amount of review it received. The amount of review is a statistic; as there is no requirement for a minimal amount of review(er)s, knowing that amount brings us no gain. The body of the commit message should summarize the consensus reached on the mailing list, That message is written as part of the patch that is reviewed; it rarely gets updated with the consensus, unless we suggest / require people to do that. However, similar to vapier's response, I'd think introducing such processes feel like unnecessary efforts. and these tags are basically standardized thank-you notes crediting non-authors who were involved in that process. They don't have to go on every patch, but if you want to mention somebody: Reviewed-by: Random J Developer ran...@developer.example.org Reviewed-by: Other R Developer ot...@developer.example.org at the end of the commit message is easier to write and read than: This patch was reviewed Random J Developer ran...@developer.example.org and Other R Developer ot...@developer.example.org. Exactly: Do we want to spend time on this or not? Do we add everyone involved? Or do we just add people whom are not on the Portage team? People in the team can be expected to be respectful and thankful, thus I prefer the latter effort (non-Portage team only) if possible. Unless we intend to introduce this for statistics, although I think that prior annotation history missing as well as people that will casually forgot to add these annotations will make the statistics a misrepresentation. At least as long as humans instead of a system add these annotations, it be more nice to have a review system that adds these for us; but well, that would be over-engineering for Portage... -- With kind regards, Tom Wijsman (TomWij) Gentoo Developer E-mail address : tom...@gentoo.org GPG Public Key : 6D34E57D GPG Fingerprint : C165 AF18 AB4C 400B C3D2 ABF0 95B2 1FCD 6D34 E57D signature.asc Description: PGP signature
Re: [gentoo-portage-dev] GitHub presence
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 18/01/14 22:52, Robin H. Johnson wrote: You mean like... https://github.com/gentoo/portage This didn't exist back when. I should have done more research though. Thanks. - -- Alexander alexan...@plaimi.net http://plaimi.net/~alexander -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlLbDVMACgkQRtClrXBQc7XXsgD/akEHqlFs2ipNGyHI0tAt3XMD V1IpKXrggIPhHrJUiD4BALNDcc0aHgkYPlf3ruzmnx0QUVqxgi+kfi5JYjWFIH3L =pXb2 -END PGP SIGNATURE-
[gentoo-portage-dev] [PATCH 3/3] emerge: Make --autounmask=y if --ask=y
--- man/emerge.1| 9 + pym/_emerge/depgraph.py | 5 +++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/man/emerge.1 b/man/emerge.1 index e23d1b4..52f8ed7 100644 --- a/man/emerge.1 +++ b/man/emerge.1 @@ -331,10 +331,11 @@ Write required unmask changes to the relevant config files, respecting \fBCONFIG_PROTECT\fR. If invoked together with \fB\-\-ask\fR, emerge will prompt you to write the changes. If invoked along with \fB\-\-pretend\fR, emerge will merely output the required changes and not make any of them by -itself. If the corresponding package.* is a file, the changes are appended to -it, if it is a directory, changes are written to the lexicographically last -file. This way it is always ensured that the new changes take precedence over -existing changes. +itself. This option is enabled by default if are running emerge with +\fB\-\-ask\fR or \fB\-\-pretend\fR, and disabled by default elsewise. If the +corresponding package.* is a file, the changes are appended to it, if it is a +directory, changes are written to the lexicographically last file. This way it +is always ensured that the new changes take precedence over existing changes. .TP .BR \-\-autounmask\-unrestricted\-atoms [ y | n ] Keyword and mask changes using the \'=\' operator will be written. With this diff --git a/pym/_emerge/depgraph.py b/pym/_emerge/depgraph.py index 5b4b723..e8b680d 100644 --- a/pym/_emerge/depgraph.py +++ b/pym/_emerge/depgraph.py @@ -6806,12 +6806,13 @@ class depgraph(object): (using CONFIG_PROTECT). The message includes the comments and the changes. - autounmask_write = self._frozen_config.myopts.get(--autounmask, n) == True + ask = --ask in self._frozen_config.myopts + autounmask_write = ask or \ + self._frozen_config.myopts.get(--autounmask, n) == True autounmask_unrestricted_atoms = \ self._frozen_config.myopts.get(--autounmask-unrestricted-atoms, n) == True quiet = --quiet in self._frozen_config.myopts pretend = --pretend in self._frozen_config.myopts - ask = --ask in self._frozen_config.myopts enter_invalid = '--ask-enter-invalid' in self._frozen_config.myopts def check_if_latest(pkg): -- 1.8.3.2
[gentoo-portage-dev] [PATCH 2/3] emerge: Rename --autounmask-write to --autounmask
Rename --autounmask-write to --autounmask. Please note that removing the option does not mean that the variable used for keeping track of autounmask writing is not removed from depgraph.py. --- man/emerge.1| 19 +++ pym/_emerge/depgraph.py | 4 ++-- pym/_emerge/main.py | 18 +- 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/man/emerge.1 b/man/emerge.1 index 58bdc94..e23d1b4 100644 --- a/man/emerge.1 +++ b/man/emerge.1 @@ -322,10 +322,20 @@ invalid input. This helps prevent accidental acceptance of the first choice. This option is intended to be set in the \fBmake.conf\fR(5) \fBEMERGE_DEFAULT_OPTS\fR variable. -\fB\-\-autounmask\-write\fR option. The +\fB\-\-autounmask\fR option. The \fBEMERGE_DEFAULT_OPTS\fR variable may be used to disable this option by default in \fBmake.conf\fR(5). .TP +.BR \-\-autounmask [ y | n ] +Write required unmask changes to the relevant config files, respecting +\fBCONFIG_PROTECT\fR. If invoked together with \fB\-\-ask\fR, emerge will +prompt you to write the changes. If invoked along with \fB\-\-pretend\fR, +emerge will merely output the required changes and not make any of them by +itself. If the corresponding package.* is a file, the changes are appended to +it, if it is a directory, changes are written to the lexicographically last +file. This way it is always ensured that the new changes take precedence over +existing changes. +.TP .BR \-\-autounmask\-unrestricted\-atoms [ y | n ] Keyword and mask changes using the \'=\' operator will be written. With this option, \'=\' operators will be used whenever possible. USE and license @@ -335,13 +345,6 @@ changes always use the latter behavior. No package.unmask or ** keyword changes will be created if this is activated. This leads to unsatisfied dependencies if no other solution exists. .TP -.BR \-\-autounmask\-write [ y | n ] -Write required unmask changes to the relevant config files, respecting -\fBCONFIG_PROTECT\fR and \fB\-\-ask\fR. If the corresponding package.* is a -file, the changes are appended to it, if it is a directory, changes are written -to the lexicographically last file. This way it is always ensured that the new -changes take precedence over existing changes. -.TP .BR \-\-backtrack=COUNT Specifies an integer number of times to backtrack if dependency calculation fails due to a conflict or an diff --git a/pym/_emerge/depgraph.py b/pym/_emerge/depgraph.py index 9fdfa43..5b4b723 100644 --- a/pym/_emerge/depgraph.py +++ b/pym/_emerge/depgraph.py @@ -6806,7 +6806,7 @@ class depgraph(object): (using CONFIG_PROTECT). The message includes the comments and the changes. - autounmask_write = self._frozen_config.myopts.get(--autounmask-write, n) == True + autounmask_write = self._frozen_config.myopts.get(--autounmask, n) == True autounmask_unrestricted_atoms = \ self._frozen_config.myopts.get(--autounmask-unrestricted-atoms, n) == True quiet = --quiet in self._frozen_config.myopts @@ -7168,7 +7168,7 @@ class depgraph(object): chk_updated_cfg_files(root, [os.path.join(os.sep, USER_CONFIG_PATH)]) elif not pretend and not autounmask_write and roots: - writemsg(\nUse --autounmask-write to write changes to config files (honoring\n + writemsg(\nUse --autounmask to write changes to config files (honoring\n CONFIG_PROTECT). Carefully examine the list of proposed changes,\n paying special attention to mask or keyword changes that may expose\n experimental or unstable packages.\n, diff --git a/pym/_emerge/main.py b/pym/_emerge/main.py index fc73ef7..247317d 100644 --- a/pym/_emerge/main.py +++ b/pym/_emerge/main.py @@ -123,9 +123,9 @@ def insert_optional_args(args): default_arg_opts = { '--ask' : y_or_n, + '--autounmask' : y_or_n, '--autounmask-keep-masks': y_or_n, '--autounmask-unrestricted-atoms' : y_or_n, - '--autounmask-write' : y_or_n, '--buildpkg' : y_or_n, '--complete-graph' : y_or_n, '--deep' : valid_integers, @@ -302,6 +302,11 @@ def parse_opts(tmpcmdline, silent=False): choices : true_y_or_n }, + --autounmask: { + help: automatically unmask packages, + choices : true_y_or_n + }, + --autounmask-unrestricted-atoms: { help: write autounmask changes with = atoms if possible, choices : true_y_or_n @@ -312,11 +317,6 @@ def
Re: [gentoo-portage-dev] Signing off patches
On Sat, 18 Jan 2014 15:24:59 -0800 W. Trevor King wk...@tremily.us wrote: If it doesn't need to get updated, then it probably already started out explaining the consensus ;). That is a guess, you can look this up in past patches. You spend time if you want to spend time and add whoever you feel moved to add. We discuss whether to make it policy to add involved people. I think the spirit of Alexander's original proposal [1] was “here is a common syntax for crediting collaborators, we might want to use it” not “ye non-conformers will be hounded unto the ends of the Earth”. Alexander is clear that best practices could be followed and that it is a proposal, note the use of the particular words should be; discussing spirits is more appropriate in another place than this mailing list. If you are submitting v2 of a patch, and feel a desire to credit reviewers / testers with this syntax, I think that's considerate of you. If you are committing someone else's patch to master, and want to record the folks who acked it on the list to distribute responsibility, that's fine too. If you want to use another syntax, or not do any of this at all, it's still fine by me ;). However, if a consistent syntax already exists, I see no reason not to use it when it suits your purpose. We discuss here whether to make it policy to use the same syntax. -- With kind regards, Tom Wijsman (TomWij) Gentoo Developer E-mail address : tom...@gentoo.org GPG Public Key : 6D34E57D GPG Fingerprint : C165 AF18 AB4C 400B C3D2 ABF0 95B2 1FCD 6D34 E57D signature.asc Description: PGP signature
[gentoo-portage-dev] [PATCH v3] Test for read-only filesystems, bail out during preinst if there are any which will be written to and display a useful error message. Fixes bug 378869.
v2: Reformat, add a function to return an appropriate read-only checker for the operating system, so that this can be extended to other OSes. v3: minor formatting tweaks from bernalex, including use os.path.join() instead of string concatenation --- pym/portage/dbapi/vartree.py | 32 + pym/portage/util/rochecker.py | 81 +++ 2 files changed, 113 insertions(+) create mode 100644 pym/portage/util/rochecker.py diff --git a/pym/portage/dbapi/vartree.py b/pym/portage/dbapi/vartree.py index ed62323..917634f 100644 --- a/pym/portage/dbapi/vartree.py +++ b/pym/portage/dbapi/vartree.py @@ -32,6 +32,7 @@ portage.proxy.lazyimport.lazyimport(globals(), 'portage.util.env_update:env_update', 'portage.util.listdir:dircache,listdir', 'portage.util.movefile:movefile', + 'portage.util.rochecker:get_ro_checker', 'portage.util._dyn_libs.PreservedLibsRegistry:PreservedLibsRegistry', 'portage.util._dyn_libs.LinkageMapELF:LinkageMapELF@LinkageMap', 'portage.util._async.SchedulerInterface:SchedulerInterface', @@ -3508,6 +3509,8 @@ class dblink(object): This function does the following: + calls get_ro_checker to retrieve a function for checking whether Portage + will write to a read-only filesystem, then runs it against the directory list calls self._preserve_libs if FEATURES=preserve-libs calls self._collision_protect if FEATURES=collision-protect calls doebuild(mydo=pkg_preinst) @@ -3685,6 +3688,7 @@ class dblink(object): eagain_error = False myfilelist = [] + mydirlist = [] mylinklist = [] paths_with_newlines = [] def onerror(e): @@ -3717,6 +3721,9 @@ class dblink(object): unicode_errors.append(new_parent[ed_len:]) break + relative_path = parent[srcroot_len:] + mydirlist.append(os.path.join(/, relative_path)) + for fname in files: try: fname = _unicode_decode(fname, @@ -3829,6 +3836,31 @@ class dblink(object): for other in others_in_slot]) prepare_build_dirs(settings=self.settings, cleanup=cleanup) + # Check for read-only filesystems + ro_checker = get_ro_checker() + rofilesystems = ro_checker(mydirlist) + + if rofilesystems: + msg = _(One or more files installed to this package are + set to be installed to read-only filesystems. + Please mount the following filesystems as read-write + and retry.) + msg = textwrap.wrap(msg, 70) + msg.append() + for f in rofilesystems: + msg.append(\t%s % os.path.join(destroot, + f.lstrip(os.path.sep))) + msg.append() + self._elog(eerror, preinst, msg) + + msg = _(Package '%s' NOT merged due to read-only file systems.) % \ + self.settings.mycpv + msg += _( If necessary, refer to your elog + messages for the whole content of the above message.) + msg = textwrap.wrap(msg, 70) + eerror(msg) + return 1 + # check for package collisions blockers = self._blockers if blockers is None: diff --git a/pym/portage/util/rochecker.py b/pym/portage/util/rochecker.py new file mode 100644 index 000..67e8131 --- /dev/null +++ b/pym/portage/util/rochecker.py @@ -0,0 +1,81 @@ +#-*- coding:utf-8 -*- +# Copyright 2014 Gentoo Foundation +# Distributed under the terms of the GNU General Public License v2 + +Methods to check whether Portage is going to write to read-only filesystems. +Since the methods are not portable across different OSes, each OS needs its +own method. To expand RO checking for different OSes, add a method which +accepts a list of directories and returns a list of mounts which need to be +remounted RW, then add elif ostype == (the ostype value for your OS) to +get_ro_checker() + +from __future__ import unicode_literals + +import logging +import re + +from portage.util import writemsg_level +from portage.localization import _ +from portage.data import ostype + + +def get_ro_checker(): + + Uses the system type to find an appropriate method for
Re: [gentoo-portage-dev] [PATCH 2/3] emerge: Rename --autounmask-write to --autounmask
On Sat, Jan 18, 2014 at 7:21 PM, Alexander Berntsen alexan...@plaimi.net wrote: Rename --autounmask-write to --autounmask. Please note that removing the option does not mean that the variable used for keeping track of autounmask writing is not removed from depgraph.py. Ok, so how do I tell portage to suggest autounmask entries without writing them to /etc/portage? Is that feature gone after your changes?
[gentoo-portage-dev] [PATCH 2/3] pym/portage/package/ebuild/fetch.py: Factor out _get_fetch_resume_size
The current fetch() function is quite long, which makes it hard to know what I can change without adverse side effects. By pulling this logic out of the main function, we get clearer logic in fetch() and more explicit input for the config extraction. --- pym/portage/package/ebuild/fetch.py | 50 - 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/pym/portage/package/ebuild/fetch.py b/pym/portage/package/ebuild/fetch.py index 4337247..bd572fa 100644 --- a/pym/portage/package/ebuild/fetch.py +++ b/pym/portage/package/ebuild/fetch.py @@ -271,6 +271,32 @@ def _get_checksum_failure_max_tries(settings, default=5): return v +def _get_fetch_resume_size(settings, default='350K'): + v = settings.get(PORTAGE_FETCH_RESUME_MIN_SIZE) + if v is not None: + v = .join(v.split()) + if not v: + # If it's empty, silently use the default. + v = default + match = _fetch_resume_size_re.match(v) + if (match is None or + match.group(2).upper() not in _size_suffix_map): + writemsg(_(!!! Variable PORTAGE_FETCH_RESUME_MIN_SIZE +contains an unrecognized format: '%s'\n) % \ + settings[PORTAGE_FETCH_RESUME_MIN_SIZE], + noiselevel=-1) + writemsg(_(!!! Using PORTAGE_FETCH_RESUME_MIN_SIZE + default value: %s\n) % default, + noiselevel=-1) + v = None + if v is None: + v = default + match = _fetch_resume_size_re.match(v) + v = int(match.group(1)) * \ + 2 ** _size_suffix_map[match.group(2).upper()] + return v + + def fetch(myuris, mysettings, listonly=0, fetchonly=0, locks_in_subdir=.locks, use_locks=1, try_mirrors=1, digests=None, allow_missing_digests=True): @@ -296,29 +322,7 @@ def fetch(myuris, mysettings, listonly=0, fetchonly=0, checksum_failure_max_tries = _get_checksum_failure_max_tries( settings=mysettings) - - fetch_resume_size_default = 350K - fetch_resume_size = mysettings.get(PORTAGE_FETCH_RESUME_MIN_SIZE) - if fetch_resume_size is not None: - fetch_resume_size = .join(fetch_resume_size.split()) - if not fetch_resume_size: - # If it's undefined or empty, silently use the default. - fetch_resume_size = fetch_resume_size_default - match = _fetch_resume_size_re.match(fetch_resume_size) - if match is None or \ - (match.group(2).upper() not in _size_suffix_map): - writemsg(_(!!! Variable PORTAGE_FETCH_RESUME_MIN_SIZE -contains an unrecognized format: '%s'\n) % \ - mysettings[PORTAGE_FETCH_RESUME_MIN_SIZE], noiselevel=-1) - writemsg(_(!!! Using PORTAGE_FETCH_RESUME_MIN_SIZE - default value: %s\n) % fetch_resume_size_default, - noiselevel=-1) - fetch_resume_size = None - if fetch_resume_size is None: - fetch_resume_size = fetch_resume_size_default - match = _fetch_resume_size_re.match(fetch_resume_size) - fetch_resume_size = int(match.group(1)) * \ - 2 ** _size_suffix_map[match.group(2).upper()] + fetch_resume_size = _get_fetch_resume_size(settings=mysettings) # Behave like the package has RESTRICT=primaryuri after a # couple of checksum failures, to increase the probablility -- 1.8.5.2.8.g0f6c0d1
[gentoo-portage-dev] [PATCH 0/3] Initial fetch() refactoring
I felt like I should actually make a useful contribution to balance the less useful commit-message discussion ;). Browsing through the open Portage bugs, #175612 looked interesting. After I looked at pym/portage/package/ebuild/fetch.py, I decided to take a step back and just try and refactor fetch(), which was pushing 1k lines. Here are three paches pulling fairly self-contained blocks out of fetch(). I thought I'd float them to the list to see if this was a productive avenue, or if this is going to be too much work to review. I tried to avoid making too many changes other than the function-extraction, but in some places I couldn't help myself ;). The patches aren't particularly well tested yet. I ran the test suite and got some errors, but they seemed to be related to my non-root invocation, and not due to these changes themselves. I thought I'd post my work so far, before digging deeper into the test suite. Cheers, Trevor [1]: https://bugs.gentoo.org/show_bug.cgi?id=175612 W. Trevor King (3): pym/portage/package/ebuild/fetch.py: Factor out _get_checksum_failure_max_tries pym/portage/package/ebuild/fetch.py: Factor out _get_fetch_resume_size pym/portage/package/ebuild/fetch.py: Factor out _get_uris pym/portage/package/ebuild/fetch.py | 305 +--- 1 file changed, 177 insertions(+), 128 deletions(-) -- 1.8.5.2.8.g0f6c0d1
[gentoo-portage-dev] [PATCH 3/3] pym/portage/package/ebuild/fetch.py: Factor out _get_uris
The current fetch() function is quite long, which makes it hard to know what I can change without adverse side effects. By pulling this logic out of the main function, we get clearer logic in fetch() and more explicit input for the config extraction. This block was especially complicated, so I also created the helper functions _get_file_uri_tuples and _expand_mirror. I'd prefer if _expand_mirror iterated through URIs instead of (group, URI) tuples, but we need a distinct marker for third-party URIs to build third_party_mirror_uris which is used to build primaryuri_dict which is used way down in fetch(): if checksum_failure_count == \ checksum_failure_primaryuri: # Switch to primaryuri mode in order # to increase the probablility of # of success. primaryuris = \ primaryuri_dict.get(myfile) if primaryuris: uri_list.extend( reversed(primaryuris)) I don't know if this is useful enough to motivate the uglier _expandmirror return values, but I'll kick that can down the road for now. --- pym/portage/package/ebuild/fetch.py | 197 +--- 1 file changed, 117 insertions(+), 80 deletions(-) diff --git a/pym/portage/package/ebuild/fetch.py b/pym/portage/package/ebuild/fetch.py index bd572fa..a617945 100644 --- a/pym/portage/package/ebuild/fetch.py +++ b/pym/portage/package/ebuild/fetch.py @@ -15,9 +15,9 @@ import sys import tempfile try: - from urllib.parse import urlparse + from urllib.parse import urlparse, urlunparse except ImportError: - from urlparse import urlparse + from urlparse import urlparse, urlunparse import portage portage.proxy.lazyimport.lazyimport(globals(), @@ -297,6 +297,118 @@ def _get_fetch_resume_size(settings, default='350K'): return v +def _get_file_uri_tuples(uris): + Return a list of (filename, uri) tuples + + file_uri_tuples = [] + # Check for 'items' attribute since OrderedDict is not a dict. + if hasattr(uris, 'items'): + for filename, uri_set in uris.items(): + for uri in uri_set: + file_uri_tuples.append((filename, uri)) + if not uri_set: + file_uri_tuples.append((filename, None)) + else: + for uri in uris: + if urlparse(uri).scheme: + file_uri_tuples.append( + (os.path.basename(myuri), myuri)) + else: + file_uri_tuples.append( + (os.path.basename(myuri), None)) + return file_uri_tuples + + +def _expand_mirror(uri, custom_mirrors=(), third_party_mirrors=()): + Replace the 'mirror://' scheme in the uri + + Returns an iterable listing expanded (group, URI) tuples, + where the group is either 'custom' or 'third-party'. + + parsed = urlparse(uri) + mirror = parsed.netloc + path = parsed.path + if path: + # Try user-defined mirrors first + if mirror in custom_mirrors: + for cmirr in custom_mirrors[mirror]: + m_uri = urlparse(cmirr) + yield ('custom', urlunparse(( + cmirr.scheme, cmirr.netloc, path) + + parsed[3:])) + + # now try the official mirrors + if mirror in third_party_mirrors: + uris = [] + for locmirr in third_party_mirrors[mirror]: + m_uri = urlparse(cmirr) + uris.append(urlunparse(( + cmirr.scheme, cmirr.netloc, path) + + parsed[3:])) + random.shuffle(uris) + for uri in uris: + yield ('third-party', uri) + else: + writemsg(_(Invalid mirror definition in SRC_URI:\n), +noiselevel=-1) + writemsg( %s\n % (uri), noiselevel=-1) + + +def _get_uris(uris, settings, custom_mirrors=(), locations=()): + third_party_mirrors = settings.thirdpartymirrors() + third_party_mirror_uris = {} + filedict = OrderedDict() + primaryuri_dict = {} + for filename, uri in _get_file_uri_tuples(uris=uris): + if filename not in filedict: + filedict[filename] = [ + os.path.join(location, 'distfiles', filename) + for location in locations] + if uri is None: + continue + if uri.startswith('mirror://'): + uris = _expand_mirror( +
Re: [gentoo-portage-dev] Signing off patches
On Sun, Jan 19, 2014 at 02:33:06AM +0100, Tom Wijsman wrote: On Sat, 18 Jan 2014 15:24:59 -0800 W. Trevor King wk...@tremily.us wrote: If it doesn't need to get updated, then it probably already started out explaining the consensus ;). That is a guess, you can look this up in past patches. Sure. Will you? If I want to touch some code, and it looks confusing, I'll use blame to see who wrote it and whay they were thinking about. If the commit message is not informative, I usually give up. I have a hard time imaging folks tracking down the thread that spawned that patch, assuming such a thread even exists, for each troublesome line they'd like to touch. It's much easier to summarize any issues the list resolved (because they're likely the same questions the new dev is asking) in the commit message, where future developers can find them. You spend time if you want to spend time and add whoever you feel moved to add. We discuss whether to make it policy to add involved people. But “involved” can be hard to pin down, especially by someone who may be applying v5 of a patch that hasn't been carefully following the whole discussion in earlier versions. The Linux kernel docs say [1]: If this patch fixes a problem reported by somebody else, consider adding a Reported-by: tag to credit the reporter for their contribution. Note the “consider” wiggle word. They are a bit more formal about Reviewed-by, but only because it's signing off on their Reviewer's statement of oversight. In general, if you're not signing some statement with the tag, formalizing “involved” is hard. If you are submitting v2 of a patch, and feel a desire to credit reviewers / testers with this syntax, I think that's considerate of you. If you are committing someone else's patch to master, and want to record the folks who acked it on the list to distribute responsibility, that's fine too. If you want to use another syntax, or not do any of this at all, it's still fine by me ;). However, if a consistent syntax already exists, I see no reason not to use it when it suits your purpose. We discuss here whether to make it policy to use the same syntax. I don't understand the distinction between “here are some guidelines, apply as and if you see fit” and “make it a policy to …”. Say you have a situation like this: 1. Alice submits a bug-report to bugs.g.o. 2. Bob codes up a Portage patch and sends it to the list. 3. Charlie responds to Bob's patch on the list with Reviewed-by. 4. Dan responds to Bob's patch on the list with Reviewed-by, and asks for any opposition. … time passes, and nobody speaks up … 5. Dan applies Bob's patch to the master branch, but neglects: Submitted-by: Alice a...@example.net Reviewed-by: Charlie c...@example.net Reviewed-by: Dan d...@example.net 6. ? As I understand it, 6 should be: 6a. Everyone gets on with their lives. I could see a situation where: 6b. Charlie reminds Dan that he could have used the tags. Everyone gets on with their lives. Is there another alternative step 6 implied by the “policy” keyword? Or is the policy workflow even more different somehow? Cheers, Trevor [1]: https://www.kernel.org/doc/Documentation/SubmittingPatches -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature