| Message ID | 20241008063325.62462-1-katariina.lounento@vaisala.com |
|---|---|
| State | New |
| Headers | show |
| Series | patchtest: add "Inactive-Upstream" | expand |
Hi Katariina, On 10/8/24 8:33 AM, Katariina Lounento via lists.openembedded.org wrote: > [You don't often get email from katariina.lounento=vaisala.com@lists.openembedded.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > From: Katariina Lounento <katariina.lounento@vaisala.com> > > The list of valid statuses (`upstream_status_literal_valid_status`) was > missing "Inactive-Upstream", which caused patchtest to fail the test > test_patch.TestPatch.test_upstream_status_presence_format for patches > containing lines like: > > +Upstream-Status: Inactive-Upstream [lastrelease: 2013 lastcommit: 2013] > > with the error: > > FAIL: test Upstream-Status presence: Upstream-Status is in incorrect format (test_patch.TestPatch.test_upstream_status_presence_format) > > "Inactive-Upstream" is documented in the Yocto Project and OpenEmbedded > Contributor Guide [1]: > > Inactive-Upstream [lastcommit: when (and/or) lastrelease: when] > > The upstream is no longer available. This typically means a > defunct project where no activity has happened for a long time — > measured in years. To make that judgement, it is recommended to > look at not only when the last release happened, but also when > the last commit happened, and whether newly made bug reports and > merge requests since that time receive no reaction. It is also > recommended to add to the patch description any relevant links > where the inactivity can be clearly seen. > I'm wondering if we simply shouldn't remove this status? I believe even if the project is inactive, we should still aim at submitting patches in the event the project starts again, or maybe it's just that nobody has sent a patch for years and the SW works good enough for all people involved. What do you think? Otherwise, looks good to me. Cheers, Quentin
On Mon, 2024-10-14 at 12:21 +0200, Quentin Schulz via lists.openembedded.org wrote: > Hi Katariina, > > On 10/8/24 8:33 AM, Katariina Lounento via lists.openembedded.org wrote: > > [You don't often get email from katariina.lounento=vaisala.com@lists.openembedded.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > > > From: Katariina Lounento <katariina.lounento@vaisala.com> > > > > The list of valid statuses (`upstream_status_literal_valid_status`) was > > missing "Inactive-Upstream", which caused patchtest to fail the test > > test_patch.TestPatch.test_upstream_status_presence_format for patches > > containing lines like: > > > > +Upstream-Status: Inactive-Upstream [lastrelease: 2013 lastcommit: 2013] > > > > with the error: > > > > FAIL: test Upstream-Status presence: Upstream-Status is in incorrect format (test_patch.TestPatch.test_upstream_status_presence_format) > > > > "Inactive-Upstream" is documented in the Yocto Project and OpenEmbedded > > Contributor Guide [1]: > > > > Inactive-Upstream [lastcommit: when (and/or) lastrelease: when] > > > > The upstream is no longer available. This typically means a > > defunct project where no activity has happened for a long time — > > measured in years. To make that judgement, it is recommended to > > look at not only when the last release happened, but also when > > the last commit happened, and whether newly made bug reports and > > merge requests since that time receive no reaction. It is also > > recommended to add to the patch description any relevant links > > where the inactivity can be clearly seen. > > > > I'm wondering if we simply shouldn't remove this status? We (as a project) have had this discussion before. There are indeed two sides to this and I can see them both. > I believe even if the project is inactive, we should still aim at > submitting patches in the event the project starts again, or maybe it's > just that nobody has sent a patch for years and the SW works good enough > for all people involved. Some of the inactive software has no place to actually visibly share patches. The other advantage to having a separate state is that it more easily allows people to focus on the areas where we can make a difference. It also gives us a big hint about which software poses a different set of risks if there is no active maintenance being done on it. Overall, I think having the state does have some benefits. I do agree we should submit where we possibly can though. Cheers, Richard
On Mon, 14 Oct 2024 at 12:21, Quentin Schulz via lists.openembedded.org <quentin.schulz=cherry.de@lists.openembedded.org> wrote: > I believe even if the project is inactive, we should still aim at > submitting patches in the event the project starts again, or maybe it's > just that nobody has sent a patch for years and the SW works good enough > for all people involved. > > What do you think? As someone (you could say, the only one) who actually does the work of submitting the existing Pending patches in oe-core and assignes Inactive-Upstream, I strongly object. Inactive-Upstream is not assigned lightly, and we do it only for projects which are clearly abandoned, and have tons of open PRs and issues that no one reacts to and in all likelihood never will. If the project starts up again (e.g. makes a release or someone discovers a promising active fork), then the patches can be easily reassigned to a status appropriate for an active project and submitted with the knowledge that someone will actually look at them and act on them. Alex
On Mon, 14 Oct 2024 at 12:21, Quentin Schulz via lists.openembedded.org <quentin.schulz=cherry.de@lists.openembedded.org> wrote: > I believe even if the project is inactive, we should still aim at > submitting patches in the event the project starts again, or maybe it's > just that nobody has sent a patch for years and the SW works good enough > for all people involved. > > What do you think? By the way, if you do want to help, then I would really appreciate if patchtest would strongly discourage people from sending Pending patches, and have an even stronger wording in place if it's Pending without an explanation in brackets. Alex
On Mon, 2024-10-14 at 12:52 +0200, Alexander Kanavin via lists.openembedded.org wrote: > On Mon, 14 Oct 2024 at 12:21, Quentin Schulz via > lists.openembedded.org > <quentin.schulz=cherry.de@lists.openembedded.org> wrote: > > I believe even if the project is inactive, we should still aim at > > submitting patches in the event the project starts again, or maybe it's > > just that nobody has sent a patch for years and the SW works good enough > > for all people involved. > > > > What do you think? > > As someone (you could say, the only one) who actually does the work of > submitting the existing Pending patches in oe-core Just to be fair to those who have helped, that isn't quite true ;-) You do a lot of it and it is much appreciated though :) Cheers, Richard
Hi Richard and Alex, On 10/14/24 13:34, Richard Purdie wrote: > On Mon, 2024-10-14 at 12:21 +0200, Quentin Schulz via lists.openembedded.org wrote: >> Hi Katariina, >> >> On 10/8/24 8:33 AM, Katariina Lounento via lists.openembedded.org wrote: >>> From: Katariina Lounento <katariina.lounento@vaisala.com> >>> >>> The list of valid statuses (`upstream_status_literal_valid_status`) was >>> missing "Inactive-Upstream", which caused patchtest to fail the test >>> test_patch.TestPatch.test_upstream_status_presence_format for patches >>> containing lines like: >>> >>> +Upstream-Status: Inactive-Upstream [lastrelease: 2013 lastcommit: 2013] >>> >>> with the error: >>> >>> FAIL: test Upstream-Status presence: Upstream-Status is in incorrect format (test_patch.TestPatch.test_upstream_status_presence_format) >>> >>> "Inactive-Upstream" is documented in the Yocto Project and OpenEmbedded >>> Contributor Guide [1]: >>> >>> Inactive-Upstream [lastcommit: when (and/or) lastrelease: when] >>> >>> The upstream is no longer available. This typically means a >>> defunct project where no activity has happened for a long time — >>> measured in years. To make that judgement, it is recommended to >>> look at not only when the last release happened, but also when >>> the last commit happened, and whether newly made bug reports and >>> merge requests since that time receive no reaction. It is also >>> recommended to add to the patch description any relevant links >>> where the inactivity can be clearly seen. >>> >> >> I'm wondering if we simply shouldn't remove this status? > > We (as a project) have had this discussion before. There are indeed two > sides to this and I can see them both. > >> I believe even if the project is inactive, we should still aim at >> submitting patches in the event the project starts again, or maybe it's >> just that nobody has sent a patch for years and the SW works good enough >> for all people involved. > > Some of the inactive software has no place to actually visibly share > patches. The other advantage to having a separate state is that it more > easily allows people to focus on the areas where we can make a > difference. It also gives us a big hint about which software poses a > different set of risks if there is no active maintenance being done on > it. > > Overall, I think having the state does have some benefits. I do agree > we should submit where we possibly can though. On 10/14/24 13:52, Alexander Kanavin wrote: > On Mon, 14 Oct 2024 at 12:21, Quentin Schulz via > lists.openembedded.org > <quentin.schulz=cherry.de@lists.openembedded.org> wrote: >> I believe even if the project is inactive, we should still aim at >> submitting patches in the event the project starts again, or maybe it's >> just that nobody has sent a patch for years and the SW works good enough >> for all people involved. >> >> What do you think? > > As someone (you could say, the only one) who actually does the work of > submitting the existing Pending patches in oe-core and assignes > Inactive-Upstream, I strongly object. > > Inactive-Upstream is not assigned lightly, and we do it only for > projects which are clearly abandoned, and have tons of open PRs and > issues that no one reacts to and in all likelihood never will. > > If the project starts up again (e.g. makes a release or someone > discovers a promising active fork), then the patches can be easily > reassigned to a status appropriate for an active project and submitted > with the knowledge that someone will actually look at them and act on > them. > > Alex Sorry for a delayed reply, but based on your replies I thought a conclusion was already reached that the inactive upstream status is useful. And regardless, I think patchtest should work according to the currently documented conventions. It seems the patch hasn't landed in openembedded-core or poky master or master-next though, so please let me know if there's anything that should be changed. (I just checked the patch still applies cleanly.) Thanks,
On 2024-10-08 02:33, Katariina Lounento via lists.openembedded.org wrote: > From: Katariina Lounento <katariina.lounento@vaisala.com> > > The list of valid statuses (`upstream_status_literal_valid_status`) was > missing "Inactive-Upstream", which caused patchtest to fail the test > test_patch.TestPatch.test_upstream_status_presence_format for patches > containing lines like: > > +Upstream-Status: Inactive-Upstream [lastrelease: 2013 lastcommit: 2013] > > with the error: > > FAIL: test Upstream-Status presence: Upstream-Status is in incorrect format (test_patch.TestPatch.test_upstream_status_presence_format) > > "Inactive-Upstream" is documented in the Yocto Project and OpenEmbedded > Contributor Guide [1]: > > Inactive-Upstream [lastcommit: when (and/or) lastrelease: when] > > The upstream is no longer available. This typically means a > defunct project where no activity has happened for a long time — > measured in years. To make that judgement, it is recommended to > look at not only when the last release happened, but also when > the last commit happened, and whether newly made bug reports and > merge requests since that time receive no reaction. It is also > recommended to add to the patch description any relevant links > where the inactivity can be clearly seen. > > `upstream_status_nonliteral_valid_status` only seems to be used in > logging and the value was copied verbatim from the aforementioned > documentation. > > After this change all upstream status options documented in [1] are > covered. > > [1] https://docs.yoctoproject.org/5.0.3/contributor-guide/recipe-style-guide.html#patch-upstream-status > > Signed-off-by: Katariina Lounento <katariina.lounento@vaisala.com> LGTM if everyone else is OK with it. As an FYI, we might see some errors with this applied on the AB for testing, as there are some ongoing issues with patches for patchtest being in master-next but not master. > --- > meta/lib/patchtest/patchtest_patterns.py | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/meta/lib/patchtest/patchtest_patterns.py b/meta/lib/patchtest/patchtest_patterns.py > index 8c2e192fc9..10d23ab77f 100644 > --- a/meta/lib/patchtest/patchtest_patterns.py > +++ b/meta/lib/patchtest/patchtest_patterns.py > @@ -83,8 +83,8 @@ patch_signed_off_by = pyparsing.AtLineStart("+" + signed_off_by_prefix + signed_ > > # upstream-status > > -upstream_status_literal_valid_status = ["Pending", "Backport", "Denied", "Inappropriate", "Submitted"] > -upstream_status_nonliteral_valid_status = ["Pending", "Backport", "Denied", "Inappropriate [reason]", "Submitted [where]"] > +upstream_status_literal_valid_status = ["Pending", "Backport", "Denied", "Inappropriate", "Submitted", "Inactive-Upstream"] > +upstream_status_nonliteral_valid_status = ["Pending", "Backport", "Denied", "Inappropriate [reason]", "Submitted [where]", "Inactive-Upstream [lastcommit: when (and/or) lastrelease: when]"] > > upstream_status_valid_status = pyparsing.Or( > [pyparsing.Literal(status) for status in upstream_status_literal_valid_status] > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#205282): https://lists.openembedded.org/g/openembedded-core/message/205282 > Mute This Topic: https://lists.openembedded.org/mt/108884475/7611679 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [tgamblin@baylibre.com] > -=-=-=-=-=-=-=-=-=-=-=- >
diff --git a/meta/lib/patchtest/patchtest_patterns.py b/meta/lib/patchtest/patchtest_patterns.py index 8c2e192fc9..10d23ab77f 100644 --- a/meta/lib/patchtest/patchtest_patterns.py +++ b/meta/lib/patchtest/patchtest_patterns.py @@ -83,8 +83,8 @@ patch_signed_off_by = pyparsing.AtLineStart("+" + signed_off_by_prefix + signed_ # upstream-status -upstream_status_literal_valid_status = ["Pending", "Backport", "Denied", "Inappropriate", "Submitted"] -upstream_status_nonliteral_valid_status = ["Pending", "Backport", "Denied", "Inappropriate [reason]", "Submitted [where]"] +upstream_status_literal_valid_status = ["Pending", "Backport", "Denied", "Inappropriate", "Submitted", "Inactive-Upstream"] +upstream_status_nonliteral_valid_status = ["Pending", "Backport", "Denied", "Inappropriate [reason]", "Submitted [where]", "Inactive-Upstream [lastcommit: when (and/or) lastrelease: when]"] upstream_status_valid_status = pyparsing.Or( [pyparsing.Literal(status) for status in upstream_status_literal_valid_status]