Governance proposal: Maintainer/code-owner model for .ci, .devcontainer, .github/workflows, tox.ini

757 views
Skip to first unread message

Matthias Koeppe

unread,
Apr 9, 2024, 5:12:00 PMApr 9
to sage-devel
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, speed up, isolate" (https://github.com/sagemath/sage/pull/36515) -- makes changes to pyrightconfig.json and src/tox.ini, so would continue to be reviewed normally


Dima Pasechnik

unread,
Apr 9, 2024, 6:28:27 PMApr 9
to sage-...@googlegroups.com


On 9 April 2024 23:11:59 CEST, Matthias Koeppe <matthia...@gmail.com> 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.

How about moving them out of the main Sage tree into separate repos, which can be accessed from the main tree as git submodules?

Then they could be developed in a completely separate process, and the corresponding PRs and issues won't be clogging up the main repo (which is already overloaded with all sorts of tangentially relevant to sagelib things.)
And the governance of these parts will be a separate thing all together.


Dima

>
>*Why the change:*

Kwankyu Lee

unread,
Apr 9, 2024, 6:36:32 PMApr 9
to sage-devel
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). 

build/bin/write-dockerfile.sh could be moved into .ci
 

Matthias Koeppe

unread,
Apr 9, 2024, 6:51:34 PMApr 9
to sage-devel
On Tuesday, April 9, 2024 at 3:28:27 PM UTC-7 Dima Pasechnik wrote:
How about moving them out of the main Sage tree into separate repos, which can be accessed from the main tree as git submodules?

That does not work. 
.github/workflows orchestrates what runs in the repo -- so it has to be in the repo.
.devcontainer declares what is offered for the repo in GitHub -- so it has to be in the repo.

Matthias Koeppe

unread,
Apr 9, 2024, 6:55:09 PMApr 9
to sage-devel
Yes, that's a plausible change. 
(But it might also be moved along with the top-level tox.ini as part of making sage-bootstrap a pip-installable package; https://github.com/sagemath/sage/issues/31662)

Kwankyu Lee

unread,
Apr 9, 2024, 7:09:00 PMApr 9
to sage-devel
Oops. Experimenting with this idea, I accidentally pushed a "crazy" commit to "develop". Please revoke the commit ASAP! 

Kwankyu Lee

unread,
Apr 9, 2024, 7:17:59 PMApr 9
to sage-devel
Oops. Experimenting with this idea, I accidentally pushed a "crazy" commit to "develop". Please revoke the commit ASAP! 

I mved ".github" directory to "github" and made a symlink ".github -> github" to see if this works, in my own repo. But I accidentally pushed to sagemath/sage! 

I tried to revoke the commit myself, but failed because "develop" is a protected branch. I don't know how I could push to the sacred branch in the first place.

Dima Pasechnik

unread,
Apr 9, 2024, 7:20:56 PMApr 9
to sage-...@googlegroups.com
Then the other way around - have a CI/sage-distro repo (which can very well have relaxed policies) with all that .github/ etc stuff needed for CI, including a part of build/ - and checkout sagelib as a submodule.

The orchestration between the two repos looks doable.


>

Matthias Koeppe

unread,
Apr 9, 2024, 8:04:48 PMApr 9
to sage-devel
On Tuesday, April 9, 2024 at 4:20:56 PM UTC-7 Dima Pasechnik wrote:
have a CI/sage-distro repo [...] with all that .github/ etc stuff needed for CI, including a part of build/ - and checkout sagelib as a submodule.

Also that does not work. Part of the .github/workflows is to run the CI on the pull requests for the Sage library, and the .devcontainer is for making GitHub Codespaces available on the pull requests for the Sage library.

Kwankyu Lee

unread,
Apr 9, 2024, 9:39:00 PMApr 9
to sage-devel
How about redefining the meaning of "CI Fix" label:

1. We designate a person to be the CI manager. 
2. For PRs pertaining to the designated directories and files, we add "CI Fix" label
3. The CI manager has the right to merge PRs with "CI Fix" label to develop.
4. The old meaning of "CI Fix" label as "immediate fixes" is dropped.
5. Hot fix PRs for breakage of CI also gets "CI Fix" label.

?


Dima Pasechnik

unread,
Apr 10, 2024, 5:06:53 AMApr 10
to sage-...@googlegroups.com
Regarding CI, triggering a git update+CI run on an event in another repo is done via a repository_dispatch hook.
<https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#repository_dispatch>

Indeed, it would be silly if one needed to pollute another repo with your CI workflow files.

Dima
>

Dima Pasechnik

unread,
Apr 10, 2024, 5:12:23 AMApr 10
to sage-...@googlegroups.com
No, this has a big risk that such a manager will accidentally push non-CI bits, and obviously the discipline for not mixing CI and non-CI bits in one commit/PR will need to be manually managed.

This is why my proposal to split the repo is much better in this way.

(One of the mottos of the political party I am forced to set up is "monorepos considered harmful", along with "batteries excluded" :-))




>
>?
>
>

Matthias Koeppe

unread,
Apr 10, 2024, 1:24:12 PMApr 10
to sage-devel
On Tuesday, April 9, 2024 at 3:28:27 PM UTC-7 Dima Pasechnik wrote:
[...] git submodules [...]

git submodules are included in a repository by specific commit sha of the submodule repo.
So whenever one has to make a change in the submodule repo, one also has to commit a change (by a second PR) in the main repo. 
Hence this does not solve anything; it only adds more friction.

git submodules are also well-known to be a developer education nightmare.
I'm pretty sure we don't want to go there as a solution for anything in the Sage project.


Matthias Koeppe

unread,
Apr 10, 2024, 1:32:57 PMApr 10
to sage-devel
Note that "CI Fix" is intended in particular for fixes to the Sage library that make the Lint workflow pass.
Such fixes are still needed and should go through normal review.

Dima Pasechnik

unread,
Apr 10, 2024, 4:00:06 PMApr 10
to sage-...@googlegroups.com


On 10 April 2024 19:24:12 CEST, Matthias Koeppe <matthia...@gmail.com> wrote:
>On Tuesday, April 9, 2024 at 3:28:27 PM UTC-7 Dima Pasechnik wrote:
>
>[...] git submodules [...]
>
>
>git submodules are included in a repository by specific commit sha of the
>submodule repo.
>So whenever one has to make a change in the submodule repo, one also has to
>commit a change (by a second PR) in the main repo.
Why a 2nd PR?

>Hence this does not solve anything; it only adds more friction.

Anyway, there are alternatives like git-subrepo https://github.com/ingydotnet/git-subrepo
and git-subtree, which are said to be
much more convenient (myself I only used git-submodule, so this is only what others say)
and don't suffer from these real or imaginary problems with git-submodule.

Dima


Matthias Koeppe

unread,
Apr 10, 2024, 4:02:29 PMApr 10
to sage-devel
On Wednesday, April 10, 2024 at 1:00:06 PM UTC-7 Dima Pasechnik wrote:
On 10 April 2024 19:24:12 CEST, Matthias Koeppe <matthia...@gmail.com> wrote:
>On Tuesday, April 9, 2024 at 3:28:27 PM UTC-7 Dima Pasechnik wrote:
>
>[...] git submodules [...]
>
>git submodules are included in a repository by specific commit sha of the
>submodule repo.
>So whenever one has to make a change in the submodule repo, one also has to
>commit a change (by a second PR) in the main repo.
Why a 2nd PR?

I just explained it. To update the version (commit sha) of the submodule.

Dima Pasechnik

unread,
Apr 10, 2024, 5:40:18 PMApr 10
to sage-...@googlegroups.com
Have you ever worked with submodules?

There is simply no need to keep the .gitsubmodules -
the only file in the main repo affected by "git submodule update --remote" -
always synchronized, commit-wise, for everyone.
Yes, it's a bit annoying to have that pesky .gitsubmodules around, and I can imagine that
git-subtree etc I mentioned are a better alternative.

Dima


Matthias Koeppe

unread,
Apr 10, 2024, 6:10:07 PMApr 10
to sage-devel
On Wednesday, April 10, 2024 at 2:40:18 PM UTC-7 Dima Pasechnik wrote:
On Wed, Apr 10, 2024 at 9:02 PM Matthias Koeppe <matthia...@gmail.com> wrote:
On Wednesday, April 10, 2024 at 1:00:06 PM UTC-7 Dima Pasechnik wrote:
On 10 April 2024 19:24:12 CEST, Matthias Koeppe <matthia...@gmail.com> wrote:
>On Tuesday, April 9, 2024 at 3:28:27 PM UTC-7 Dima Pasechnik wrote:
>[...] git submodules [...]
>
>git submodules are included in a repository by specific commit sha of the
>submodule repo.
>So whenever one has to make a change in the submodule repo, one also has to
>commit a change (by a second PR) in the main repo.
Why a 2nd PR?

I just explained it. To update the version (commit sha) of the submodule.

Have you ever worked with submodules?

Yes, Dima, I have.
 
There is simply no need to keep the .gitsubmodules -
the only file in the main repo affected by "git submodule update --remote" -
always synchronized, commit-wise, for everyone.

There is. Without doing that, the changes made in the submodule won't take effect. 
That's the whole point.

Dima Pasechnik

unread,
Apr 10, 2024, 6:25:16 PMApr 10
to sage-...@googlegroups.com
This is not true.

  git submodule update --remote

will pick up submodules changes just fine.
Read the docs?

Matthias Koeppe

unread,
Apr 10, 2024, 7:23:13 PMApr 10
to sage-devel
Necessary reminder that we're discussing, as the subject says, the files that control the GitHub workflows and the Codespaces.
What a developer may do on their local machine does not matter.

dim...@gmail.com

unread,
Apr 11, 2024, 7:28:12 AMApr 11
to sage-...@googlegroups.com
But there is no difference here between a CI and a local machine here.
A CI is perfectly capable of doing
"git submodule update --remote"
and proceed.


>
> --
> 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/b178151b-6c80-4a8c-a93d-04fc824f969dn%40googlegroups.com.

signature.asc

Matthias Koeppe

unread,
Apr 11, 2024, 12:12:26 PMApr 11
to sage-devel
On Thursday, April 11, 2024 at 4:28:12 AM UTC-7 dim...@gmail.com wrote:
On Wed, Apr 10, 2024 at 04:23:13PM -0700, Matthias Koeppe wrote:
> On Wednesday, April 10, 2024 at 3:25:16 PM UTC-7 Dima Pasechnik wrote:
> Necessary reminder that we're discussing, as the subject says, the files
> that control the GitHub workflows and the Codespaces.
> What a developer may do on their local machine does not matter.

But there is no difference here between a CI and a local machine here.
A CI is perfectly capable of doing
"git submodule update --remote"
and proceed.

No. These files are processed by GitHub before any custom code can be run.

Dima Pasechnik

unread,
Apr 11, 2024, 3:36:20 PMApr 11
to sage-...@googlegroups.com
Are you saying that "git submodule..." cannot be triggered by repository_dispatch hook?
So GH Actions cannot be triggered by a submodule update?



>

Matthias Koeppe

unread,
Apr 11, 2024, 3:52:41 PMApr 11
to sage-devel
Once again, the workflow files in .github/workflows have to be statically present in a repository so that Actions work.
And the .devcontainers files have to be statically present in a repository so that Codespaces work. 

If you want to propose a design for breaking our repository into multiple repositories, work it out first. Learn about the relevant technological restrictions. 
It does not make sense to develop it here in a question-and-answer game. 
In particular, this cannot be done here in this thread about governance; it's only a distraction.

Matthias

Dima Pasechnik

unread,
Apr 11, 2024, 4:15:52 PMApr 11
to sage-...@googlegroups.com


On 11 April 2024 21:47:57 CEST, Matthias Koeppe <matthia...@gmail.com> wrote:
>Once again, the workflow files in .github/workflows have to be statically
>present in a repository so that Actions work.
>And the .devcontainers files have to be statically present in a repository
>so that Codespaces work.

Yes, sure, as I said, you can have it statically present, and test a submodule containing sagelib from there.

There is nothing hard to design here, once you split the repo this way.

Matthias Koeppe

unread,
Apr 11, 2024, 4:46:06 PMApr 11
to sage-devel
I will not respond further on this in this thread.

Matthias Koeppe

unread,
Apr 21, 2024, 4:25:39 AMApr 21
to sage-devel
I'm making this suggested change in https://github.com/sagemath/sage/pull/37841

Matthias Koeppe

unread,
May 6, 2024, 4:56:53 PMMay 6
to sage-devel
Dear Sage developers:

I include an updated proposal below, with changes brought by the merged https://github.com/sagemath/sage/pull/37841, a clarification, and updated examples).
I ask everyone to focus the discussion on the specifics of the proposal.
I plan to call a vote on this in a week or so.

Matthias

=== Proposal (v2)

I propose a governance change for a small part of the main 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, as well as the Dev Containers feature on GitHub.
2. The file tox.ini. It contains the infrastructure for portability testing of the Sage distribution (https://doc.sagemath.org/html/en/developer/portability_testing.html).
3. The file src/doc/en/developer/portability_platform_table.rst (which I update using "tox -e update_docker_platforms").

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. In particular, some of the open PRs listed as examples in the original post have been merged; thanks, Kwankyu, for reviewing of all of them!

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.

Examples (all PRs authored by me, waiting for review):
- "dist.yml: Download optional/experimental tarballs for GitHub Release assets" (https://github.com/sagemath/sage/pull/37762)
- "CI: Handle the 'p: CI Fix' label" (https://github.com/sagemath/sage/pull/37950)

Non-examples (all PRs authored by me, waiting for review):
- "tox.ini: Add environments ruff, ruff-minimal; GH Actions: run ruff-minimal" (https://github.com/sagemath/sage/pull/37453) -- this also makes changes in src/tox.ini and src/doc, so would continue to be reviewed normally
- "sage -tox -e pyright: Update, speed up, isolate" (https://github.com/sagemath/sage/pull/36515) -- makes changes to pyrightconfig.json and src/tox.ini, so would continue to be reviewed normally

Kwankyu Lee

unread,
May 7, 2024, 1:49:07 AMMay 7
to sage-devel
I propose a governance change for a small part of the main 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, as well as the Dev Containers feature on GitHub.
2. The file tox.ini. It contains the infrastructure for portability testing of the Sage distribution (https://doc.sagemath.org/html/en/developer/portability_testing.html). 
3. The file src/doc/en/developer/portability_platform_table.rst (which I update using "tox -e update_docker_platforms").

I think we should restrict the scope to the files and directories strictly in the CI infrastructure.  Anything under `src/` should be excluded.

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.

I fear that developers in the Maintainer role could conflict in making changes to the CI-related files, as we suffered in the disputed PRs. So I suggest to have only one Maintainer for those files. As we acknowledge a certain dictatorship of the release manager, we may have a vote to elect the CI manager and give him/her a restricted dictatorship and responsibility.

julian...@fsfe.org

unread,
May 8, 2024, 9:20:52 AMMay 8
to sage-devel
Hi Matthias,

It was a bit unclear to me how your v2 proposal is different from the initial proposal on this sage-devel thread. Maybe it's helpful to clarify that build/bin/write-dockerfile.sh was removed from the proposal and src/doc/en/developer/portability_platform_table.rst was added. Otherwise, it's in essentially unchanged.

julian

Matthias Koeppe

unread,
May 8, 2024, 2:23:57 PMMay 8
to sage-devel
On Wednesday, May 8, 2024 at 6:20:52 AM UTC-7 julian...@fsfe.org wrote:
It was a bit unclear to me how your v2 proposal is different from the initial proposal on this sage-devel thread. Maybe it's helpful to clarify that build/bin/write-dockerfile.sh was removed from the proposal and src/doc/en/developer/portability_platform_table.rst was added. Otherwise, it's in essentially unchanged.

That's right; as I wrote, "changes brought by the merged https://github.com/sagemath/sage/pull/37841, a clarification, and updated examples".

Matthias Koeppe

unread,
May 8, 2024, 2:25:54 PMMay 8
to sage-devel
On Monday, May 6, 2024 at 10:49:07 PM UTC-7 Kwankyu Lee wrote:
I propose a governance change for a small part of the main Sage repository:
1. The directories .ci, .devcontainer, .github/workflows. [...]
2. The file tox.ini. [...]
3. The file src/doc/en/developer/portability_platform_table.rst (which I update using "tox -e update_docker_platforms").

I think we should restrict the scope to the files and directories strictly in the CI infrastructure.  Anything under `src/` should be excluded.

This file src/doc/en/developer/portability_platform_table.rst is part of the documentation of the CI infrastructure and needs to be updated (by script) whenever I make changes to the tested platforms. 
 

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.

I fear that developers in the Maintainer role could conflict in making changes to the CI-related files, as we suffered in the disputed PRs.

I don't share this concern. 

By the way, I documented who is currently in the Maintainer role when I prepared the NumFOCUS application last month, see https://github.com/sagemath/sage/wiki/NumFOCUS#q13new-please-list-your-projects-maintainers-ie-anyone-with-write-access-to-the-repository
(I haven't checked if it has changed since.)

There's certainly a broader governance discussion that we should have at some point (regarding duties and appointment procedures for the Maintainer role, the Triage team, and other functions in the project), but I would suggest to do this in a separate thread.

So I suggest to have only one Maintainer for those files. As we acknowledge a certain dictatorship of the release manager, we may have a vote to elect the CI manager and give him/her a restricted dictatorship and responsibility.

This model would also work for me, but I think it's unnecessarily specific.

Dima Pasechnik

unread,
May 8, 2024, 3:18:51 PMMay 8
to sage-...@googlegroups.com
I've already said while the previous version of this was discussed, is
that it's a huge mess to have different commit rights for different
parts of the tree, and I proposed to spin the CI into a separate
repository, as an alternative which simplifies a lot of things.

Dima
> --
> 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/e955c1d9-96f5-495d-85a9-9267f5414d3dn%40googlegroups.com.

Kwankyu Lee

unread,
May 8, 2024, 9:51:49 PMMay 8
to sage-devel
On Thursday, May 9, 2024 at 3:25:54 AM UTC+9 Matthias Koeppe wrote:
On Monday, May 6, 2024 at 10:49:07 PM UTC-7 Kwankyu Lee wrote:
I propose a governance change for a small part of the main Sage repository:
1. The directories .ci, .devcontainer, .github/workflows. [...]
2. The file tox.ini. [...]
3. The file src/doc/en/developer/portability_platform_table.rst (which I update using "tox -e update_docker_platforms").

I think we should restrict the scope to the files and directories strictly in the CI infrastructure.  Anything under `src/` should be excluded.

This file src/doc/en/developer/portability_platform_table.rst is part of the documentation of the CI infrastructure and needs to be updated (by script) whenever I make changes to the tested platforms. 

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?  

If the answer is no, then  .devcontainer and src/doc/en/developer/portability_platform_table.rst can be excluded from the CI manager governance. 
 

Volker Braun

unread,
May 9, 2024, 5:18:41 AMMay 9
to sage-devel
+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

Matthias Koeppe

unread,
May 9, 2024, 12:04:27 PMMay 9
to sage-devel
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.  

Matthias Koeppe

unread,
May 10, 2024, 10:02:38 AMMay 10