Re: [sage-devel] Re: New labels v: mimimal, v: small ... on pull requests

[seb....@gmail.com]

I must confess that I have not thought about the aspects of these labels 
that Travis points out, but I fully understand these concerns. If they are 
annoying for many developers, the feature can be easily disabled by 
removing corresponding variables from the repository.


Vincent Delecroix schrieb am Freitag, 10. Mai 2024 um 07:45:13 UTC+2:

> I fully agree with Travis. I do not see the added value of these
> additional tags.
>
> On Thu, 9 May 2024 at 23:46, Travis Scrimshaw  wrote:
> >
> > I am *very* strongly opposed to these tags. Their cutoffs are arbitrary 
> nor they serve no useful purpose as far as I can tell. To this point, they 
> do not reflect the difficulty of a review; in fact, they are at best 
> counterproductive to finding reviewers because it might deter people from 
> reviewing "large" or "huge" changes as they can include lots of trivial 
> doctest changes. At best it is just additional clutter in all of the 
> information for PRs.
> >
> > From a community perspective, I feel such changes should have been 
> brought to the attention of sage-devel once the PR was at a positive 
> review. Specifically, before the PR was merged. Not everyone has time to 
> read every PR, and a small consensus of developers might not reflect the 
> development community at-large when making changes like this.
> >
> > Best,
> > Travis
> >
> >
> > On Tuesday, May 7, 2024 at 3:12:27 PM UTC+9 seb@gmail.com wrote:
> >>
> >> Dear Sage developers,
> >>
> >> You may have noticed that since yesterday a new type of labels with the 
> `v:` prefix has appeared on our PRs. These are automatically set to 
> classify PRs based on their size. For more information, see #37262.
> >>
> >> Sebastian
> >
> > --
> > You received this message because you are subscribed to the Google 
> Groups "sage-devel" group.
> > To unsubscribe from this group and stop receiving emails from it, send 
> an email to sage-devel+...@googlegroups.com.
> > To view this discussion on the web visit 
> https://groups.google.com/d/msgid/sage-devel/a1904934-a31f-4ce0-83c2-76cf2d1a70f1n%40googlegroups.com
> .
>

-- 
You received this message because you are subscribed to the Google Groups 
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to sage-devel+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/sage-devel/d09d6718-ba19-4420-9b94-2fdd722bdc72n%40googlegroups.com.

Re: [sage-devel] Re: New labels v: mimimal, v: small ... on pull requests

[Vincent Delecroix]

I fully agree with Travis. I do not see the added value of these
additional tags.

On Thu, 9 May 2024 at 23:46, Travis Scrimshaw  wrote:
>
> I am *very* strongly opposed to these tags. Their cutoffs are arbitrary nor 
> they serve no useful purpose as far as I can tell. To this point, they do not 
> reflect the difficulty of a review; in fact, they are at best 
> counterproductive to finding reviewers because it might deter people from 
> reviewing "large" or "huge" changes as they can include lots of trivial 
> doctest changes. At best it is just additional clutter in all of the 
> information for PRs.
>
> From a community perspective, I feel such changes should have been brought to 
> the attention of sage-devel once the PR was at a positive review. 
> Specifically, before the PR was merged. Not everyone has time to read every 
> PR, and a small consensus of developers might not reflect the development 
> community at-large when making changes like this.
>
> Best,
> Travis
>
>
> On Tuesday, May 7, 2024 at 3:12:27 PM UTC+9 seb@gmail.com wrote:
>>
>> Dear Sage developers,
>>
>> You may have noticed that since yesterday a new type of labels with the `v:` 
>> prefix has appeared on our PRs. These are automatically set to classify PRs 
>> based on their size. For more information, see #37262.
>>
>> Sebastian
>
> --
> You received this message because you are subscribed to the Google Groups 
> "sage-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to sage-devel+unsubscr...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/sage-devel/a1904934-a31f-4ce0-83c2-76cf2d1a70f1n%40googlegroups.com.

-- 
You received this message because you are subscribed to the Google Groups 
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to sage-devel+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/sage-devel/CAGEwAA%3DBaLG%2B4BfJ-qw9cJKY5x_4zKc1%2B76yF%2BNQ6MJLVV3CMw%40mail.gmail.com.

[sage-devel] Re: New labels v: mimimal, v: small ... on pull requests

[Travis Scrimshaw]

I am *very* strongly opposed to these tags. Their cutoffs are arbitrary nor 
they serve no useful purpose as far as I can tell. To this point, they do 
not reflect the difficulty of a review; in fact, they are at best 
counterproductive to finding reviewers because it might deter people from 
reviewing "large" or "huge" changes as they can include lots of trivial 
doctest changes. At best it is just additional clutter in all of the 
information for PRs.

>From a community perspective, I feel such changes should have been brought 
to the attention of sage-devel once the PR was at a positive review. 
Specifically, *before* the PR was merged. Not everyone has time to read 
every PR, and a small consensus of developers might not reflect the 
development community at-large when making changes like this.

Best,
Travis


On Tuesday, May 7, 2024 at 3:12:27 PM UTC+9 seb@gmail.com wrote:

> Dear Sage developers,
>
> You may have noticed that since yesterday a new type of labels with the 
> `v:` prefix has appeared on our PRs. These are automatically set to 
> classify PRs based on their size. For more information, see #37262 
> .
>
> Sebastian
>

-- 
You received this message because you are subscribed to the Google Groups 
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to sage-devel+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/sage-devel/a1904934-a31f-4ce0-83c2-76cf2d1a70f1n%40googlegroups.com.

[sage-devel] Re: Governance proposal: Maintainer/code-owner model for .ci, .devcontainer, .github/workflows, tox.ini

[Matthias Koeppe]

On Wednesday, May 8, 2024 at 6:51:49 PM UTC-7 Kwankyu Lee wrote:

The command  "tox -e update_docker_platforms" involves a change of the list 
of tested platforms. The change become effective when a beta release is 
made by the release manager, since the docker image files are created only 
at the release time. So there is no need to merge early the change to the 
develop branch. 

On the other hand, is it an unnecessary friction that changes of the list 
of tested platforms go through the normal review process?  


Yes, that's part of the friction that I would like to eliminate. The 
function of merging such platform updates is to get the infrastructure in 
place that allows us to see whether the release is ready to be made. But 
for example the last platform update 
(https://github.com/sagemath/sage/pull/37351) missed the 10.3 release, 
taking 2 months to get merged.  

-- 
You received this message because you are subscribed to the Google Groups 
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to sage-devel+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/sage-devel/c1c8d875-37b0-44f1-8319-4f3e1bba9831n%40googlegroups.com.

[sage-devel] Re: Governance proposal: Maintainer/code-owner model for .ci, .devcontainer, .github/workflows, tox.ini

[Volker Braun]

+1 to the general idea of maintainers for distinct subtrees 

As far as the implementation, I'd rather follow a model where there is a 
single merge queue at the end (currently me, could be automated when the CI 
is stricter and developers do not / cannot ignore it any more). Otherwise 
we'll just either have broken releases (untested rebase at the end) or 
drain my time by having to rebase / retest (takes a long time on the 
buildbot) / notice than another rebase is needed / rinse and repeat

Really all you need is to 
* have a "CI Build & Test" tag whose PR's are merged in before CI runs. 
* maintainer sets positive review on tickets under his responsibility
On Tuesday, April 9, 2024 at 11:12:00 PM UTC+2 Matthias Koeppe wrote:

> Dear Sage developers:
> I propose to consider the following governance change for a small part of 
> the Sage repository:
> 1. The directories *.ci, .devcontainer, .github/workflows*. These are 
> special directories that control the GitHub workflows that run for example 
> on pull requests and when release tags are pushed.
> 2. The files *tox.ini* and *build/bin/write-dockerfile.sh*. They contain 
> the infrastructure for portability testing of the Sage distribution (
> https://doc.sagemath.org/html/en/developer/portability_testing.html). 
>
> Some of these files are shipped as part of the Sage distribution, but none 
> of them have any role in the build process or runtime of Sage, and thus 
> none of them are tested by the Release Manager.
>
> *Status quo: *All changes to these files go through the normal review 
> process for Sage PRs; when set to "positive review", Volker merges them 
> into the next development release.
> In the terminology of https://martinfowler.com/articles/ship-show-ask.html 
> (ht Gonzalo Tornaria), this is the "Ask" model.
>
> Acknowledgment: I'm grateful to all who have contributed to the review of 
> my PRs that made changes to these files in the past: thanks for your time 
> and energy.
>
> *Proposed change: *All changes to these files are made through PRs. When 
> the PR is ready, a developer in the Maintainer role directly merges the PR 
> into the "develop" branch.
> In other words, switch to the "Show" model for these changes.
>
> *Why the change:*
> 1. Changes to these files do not have any effect on the build and runtime 
> of Sage;
> - thus changes to these files do not risk breaking the mathematical 
> correctness, or the performance of anything in Sage;
> - hence there may not be the same need for formal review compared to 
> changes to the Sage library.
>
> 2. Our project has a collective interest in smoothly operating development 
> infrastructure / quality assurance tools;
> - but tragedy of the commons;
> - more specifically, developing/improving such development tools only pays 
> off individually for developers with a sufficiently high volume of activity 
> (cf. 
> https://github.com/sagemath/sage/graphs/contributors?from=2020-01-01&to=2024-04-09&type=c
> );
> - there may also be a technical barrier that prevents developers from even 
> reviewing a PR that makes changes to these files;
> - hence, waiting for reviewers to approve a PR and waiting for the Release 
> Manager to merge it adds too much delay and friction.
>
> 3. Examples (all PRs authored by me, waiting for review):
> - "CI build, doc-build: Run containers explicitly, separate jobs for 
> pyright, build, modularized tests, long tests" (
> https://github.com/sagemath/sage/pull/36498) waiting for review since Oct 
> 21, 2023
> - "GH Actions: Build platform-independent wheels of sagemath-environment, 
> sage-setup, sage-sws2rst for PyPI" (
> https://github.com/sagemath/sage/pull/37099) waiting for review since Feb 
> 5
> - "CI: Update Linux platforms / runners" waiting for review since Feb 14
> - "GH Actions: Build macOS arm64 wheels" (
> https://github.com/sagemath/sage/pull/37503) waiting for review since Feb 
> 28
> - "CI Build&Test: Fix "test modularized distributions" (
> https://github.com/sagemath/sage/pull/37750) waiting for review since Apr 
> 4
> - "dist.yml: Download optional/experimental tarballs for GitHub Release 
> assets" (https://github.com/sagemath/sage/pull/37762) waiting for review 
> since Apr 6
>
> 4. Non-examples (all PRs authored by me, waiting for review):
> - "CI Build&Test: Show segfaults using GitHub annotations" (
> https://github.com/sagemath/sage/pull/37738, waiting for review) -- this 
> makes changes in sage.doctest, so would continue to be reviewed normally
> - "tox.ini: Add environments ruff, ruff-minimal; GH Actions: run 
> ruff-minimal" (https://github.com/sagemath/sage/pull/37453, waiting for 
> review) -- this also makes changes in src/tox.ini and src/doc, so would 
> continue to be reviewed normally
> - "src/tox.ini (coverage:run): Set concurrency = multiprocessing,threads" (
> https://github.com/sagemath/sage/pull/37010) -- makes changes in 
> src/tox.ini, so would continue to be reviewed normally
> - "sage -tox -e pyright: Update, sp