Unload "blocker" label

[Kwankyu Lee]

Hi

"blocker" label is overloaded too much. It is used for

usage 1: PRs that should be merged to the next release
usage 2: PRs that are merged temporarily before CI tests run
usage 3: Issues that should be fixed as fast as possible

I suggest the following:

Use "blocker" label only for PRs and use "critical" instead for Issues.

If there is no objection in a couple of days, then I will

(a) change all "blocker" labels on Issues to "critical" labels
(b) remove all "critical" labels from old Issues converted from trac

Thanks for attention.

[Tobia...@gmx.de]

Just move "usage 2" to a new label. Would be more intuitive and explicit in my opinion.

[Kwankyu Lee]

On Monday, February 26, 2024 at 4:46:36 PM UTC+9 Tobia...@gmx.de wrote:

Just move "usage 2" to a new label. Would be more intuitive and explicit in my opinion.

I am a bit inclined to your opinion, but not sure. Others may argue that "usage 1" and " usage 2" are better to be combined under one label.

Anyway, here I am focusing on the (easy) issue of separating "usage 3" from the "blocker" label. 

 

[Vincent Delecroix]

Hi Kwankyu,

I do not agree with

usage 3: Issues that should be fixed as fast as possible

To me it is rather "issues that should be fixed before the next
release" (or at least it was the way it was supposed to work when we
had trac). This looks better to me as that there is no reason to
release a broken sage.

Best
Vincent

> --
> 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/609bd8c5-92dd-4c64-8204-a4956517558an%40googlegroups.com.

[Dima Pasechnik]

On 26 February 2024 09:08:08 GMT, Vincent Delecroix <20100.d...@gmail.com> wrote:
>Hi Kwankyu,
>
>I do not agree with
>
>usage 3: Issues that should be fixed as fast as possible
>
>To me it is rather "issues that should be fixed before the next
>release" (or at least it was the way it was supposed to work when we
>had trac). This looks better to me as that there is no reason to
>release a broken sage.

Indeed, +1. Issues to be fixed "as fast as possible" are only critical, or less.

"blocker" normally means "blocking the release, must be fixed".

[Kwankyu Lee]

On Monday, February 26, 2024 at 6:49:35 PM UTC+9 Dima Pasechnik wrote:

>usage 3: Issues that should be fixed as fast as possible
>
>To me it is rather "issues that should be fixed before the next
>release" (or at least it was the way it was supposed to work when we
>had trac). This looks better to me as that there is no reason to
>release a broken sage.

We are now on github

 

Indeed, +1. Issues to be fixed "as fast as possible" are only critical, or less.

"blocker" normally means "blocking the release, must be fixed".

Then these blocker Issues with no corresponding PRs

https://github.com/sagemath/sage/issues/37263

https://github.com/sagemath/sage/issues/36914

forbid the release manager to make a release?

Or (as I propose) only blocker PRs fixing the "blocker" Issues forbid the release manager to make a release?

 

[Dima Pasechnik]

On Mon, Feb 26, 2024 at 11:36 AM Kwankyu Lee <ekwa...@gmail.com> wrote:

On Monday, February 26, 2024 at 6:49:35 PM UTC+9 Dima Pasechnik wrote:

>usage 3: Issues that should be fixed as fast as possible
>
>To me it is rather "issues that should be fixed before the next
>release" (or at least it was the way it was supposed to work when we
>had trac). This looks better to me as that there is no reason to
>release a broken sage.

We are now on github

 

Indeed, +1. Issues to be fixed "as fast as possible" are only critical, or less.

"blocker" normally means "blocking the release, must be fixed".

Then these blocker Issues with no corresponding PRs

https://github.com/sagemath/sage/issues/37263

https://github.com/sagemath/sage/issues/36914

forbid the release manager to make a release?

I don't know about these particular issues, but a scenario where things are too broken for a release (e.g. docs stopped building) might happen, and did happen in the part. Are you saying that only PRs can block a release?

But how does one even report a very serious issue, without offering a ready fix? 

Are you saying one should use other channels of communication for this? (Which is weird, to say the least).

Yes, with a good probability, such a huge breakage will be noticed by Volker, but what if not? Or for quite some time such a breaking issue won't be noticed?

 

Or (as I propose) only blocker PRs fixing the "blocker" Issues forbid the release manager to make a release?

 

--

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/0638e722-ddb2-4112-a721-7a25c11b5882n%40googlegroups.com.

[Emmanuel Charpentier]

Le lundi 26 février 2024 à 12:59:47 UTC+1, Dima Pasechnik a écrit :

On Mon, Feb 26, 2024 at 11:36 AM Kwankyu Lee <ekwa...@gmail.com> wrote:

On Monday, February 26, 2024 at 6:49:35 PM UTC+9 Dima Pasechnik wrote:

>usage 3: Issues that should be fixed as fast as possible
>
>To me it is rather "issues that should be fixed before the next
>release" (or at least it was the way it was supposed to work when we
>had trac). This looks better to me as that there is no reason to
>release a broken sage.

We are now on github

 

Indeed, +1. Issues to be fixed "as fast as possible" are only critical, or less.

"blocker" normally means "blocking the release, must be fixed".

Then these blocker Issues with no corresponding PRs

https://github.com/sagemath/sage/issues/37263

https://github.com/sagemath/sage/issues/36914

forbid the release manager to make a release?

I don't know about these particular issues, but a scenario where things are too broken for a release (e.g. docs stopped building) might happen, and did happen in the part. Are you saying that only PRs can block a release?

No ! (Catastrophic) issues should be able to block a release (see below)

But how does one even report a very serious issue, without offering a ready fix? 

This is extremely important : having a way to allow a “plain Sage user“ (i. e. someone that “just uses Sage” without being a developer nor having a Github account) is a very important feature; We had this in trac (I can’t rememeber how, but I remember starting fiddling with Sage this way), and lost it when switching to Github. Currently, the only way an ordinary user can report an issue is to wail in sage-support and pray for a kind developer soul to create the relevant issue(s).

Back to labels : the confusing part is that, as far as I understand, labels are used to qualify both PRs (e. g. needs_review, needs work and issues (e. g. minor, major, critical, blocker).

Both uses are necessary. But the wording may need a bit of reworking. In the case of blocker, there are two possible uses :

• Issue : the report documents a case where Sage gives a seriuosly wrong answer, never returns or crashes (I’ve seen al these cases).

• PR : the fixer or the reviewer report a case where some change in Sagemath must be implemented in the next release (or in the next beta : we might distinguish those two cases ?) in order to keep, say, consistency or convention, or allow use of other (present or future) parts of Sage.

Similar distinct use cases come to mind for other labels, such as minor or major). Unless there is an alternative to the labels mechanism, we should have at least distinct labels for PR and issue usages.

Are you saying one should use other channels of communication for this? (Which is weird, to say the least).

No ! (See above…).

HTH;

​

[Kwankyu Lee]

But how does one even report a very serious issue, without offering a ready fix? 

I am proposing to use "critical" label for that purpose. That is why I also proposed to remove "critical" labels from old Issues (converted from trac tickets). 

[Kwankyu Lee]

Anyway, as there are only objections here, I give up.

Thanks for opinions.

[Dima Pasechnik]

On Mon, Feb 26, 2024 at 12:45 PM Emmanuel Charpentier <emanuel.c...@gmail.com> wrote:

Le lundi 26 février 2024 à 12:59:47 UTC+1, Dima Pasechnik a écrit :

[ Snip... ]

 Are you saying that only PRs can block a release?

But how does one even report a very serious issue, without offering a ready fix? 

Are you saying one should use other channels of communication for this? (Which is weird, to say the least).

*Note :* having a way to allow a "plain Sage *user*" (i. e. someone that "just uses Sage" without being a developer nor having a Github account) is a very important feature; We had this in `trac`, and lost it when switching to Github. Currently, the only way an ordinary user can report an issue is to wail in `sage-support` and pray for a kind developer soul to create the relevant issue(s). 

well, 99.9 % of our users either already have a GitHub account, or can create one rather easily.

 

Back to labels : the confusing part is that, as far as I understand, labels are used to qualify both *PRs* (e. g. `needs_review`, `needs work` and *issues* (e. g. `minor`, `major`, `critical`, `blocker`).

*Both* uses are necessary. But the wording may need a bit of reworking. In the case of `blocker`, there are two possible uses :

-  Issue : the report documents a case where Sage gives a seriuosly wrong answer, never returns or crashes (I've seen al these cases).

- PR : the fixer or the reviewer report a case where some change in Sagemath *must* be implemented in the next release (or in the next beta : we might distinguish those two cases ?) in order to keep, say, consistency or convention, or allow use of other (present or future) parts of Sage.

Similar distinct use cases come to mind for other labels, such as `minor` or `major`). Unless there is an alternative to the `labels` mechanism, we should have at least distinct labels for `PR` and `issue` usages.

 

Yes, with a good probability, such a huge breakage will be noticed by Volker, but what if not? Or for quite some time such a breaking issue won't be noticed?

 

Or (as I propose) only blocker PRs fixing the "blocker" Issues forbid the release manager to make a release?

 

--

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/0638e722-ddb2-4112-a721-7a25c11b5882n%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+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/bb7f9798-9456-424c-80f2-8f863ac5bcb5n%40googlegroups.com.

[Vincent Delecroix]

Dear Kwankyu,

Note that everybody who kindly took the time to consider your proposal
responded the same : it seems more consistent to have only two
categories {1, 3} and {2} rather than three (following your
numbering).

Either you give up because people disagree with you (which is a
problem about yourself) or you feel like your proposal was not given
the credit it should have been (which is a problem with the tone of
the "commenters" e-mails). Or maybe something else. But sending an
e-mail saying that you give up because of objections is neither
helping the discussion nor the technical problem that you pointed out
in your initial e-mail.

Best
Vincent

On Mon, 26 Feb 2024 at 14:31, Kwankyu Lee <ekwa...@gmail.com> wrote:
>
> Anyway, as there are only objections here, I give up.
>
> Thanks for opinions.
>

> --
> 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/6b53b2d5-7d52-479b-8886-50c8f1100637n%40googlegroups.com.

[Matthias Koeppe]

On Monday, February 26, 2024 at 5:31:47 AM UTC-8 Kwankyu Lee wrote:

Anyway, as there are only objections here, I give up.

Thanks for opinions.

On Monday, February 26, 2024 at 6:02:41 AM UTC-8 Vincent Delecroix wrote:

Dear Kwankyu,

Either you give up because people disagree with you (which is a
problem about yourself) or you feel like your proposal was not given
the credit it should have been (which is a problem with the tone of
the "commenters" e-mails). Or maybe something else. But sending an
e-mail saying that you give up because of objections is neither
helping the discussion nor the technical problem that you pointed out
in your initial e-mail.

Vincent, I think both are unnecessarily harsh readings of "I give up."

I read Kwankyu's post simply as "I am withdrawing my proposal".

[Vincent Delecroix]

In that case, let me do a proposal.

Introduce a new label distinct from "blocker" for

usage 2: PRs that should be merged temporarily before CI tests run

(even though I think that "merged temporarily" would better be clarified)

Vincent

> --
> 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/0857b65d-3e62-49c2-a622-d692d836bce2n%40googlegroups.com.

[Matthias Koeppe]

On Monday, February 26, 2024 at 9:43:18 AM UTC-8 Vincent Delecroix wrote:

let me do a proposal.

Introduce a new label distinct from "blocker" for

usage 2: PRs that should be merged temporarily before CI tests run

For reference, this proposal is the same as https://github.com/sagemath/sage/issues/37428

I don't have a strict objection to it.

But let me take this opportunity to share a bit of background on this mechanism, which I introduced in https://github.com/sagemath/sage/pull/36338, and the design choice to reuse the "blocker" label for it. 

(So far this workflow feature has only been documented in https://github.com/sagemath/sage/wiki/Sage-10.2-Release-Tour#open-blocker-prs-are-applied-automatically-in-ci-workflows

and a change to it in https://github.com/sagemath/sage/wiki/Sage-10.3-Release-Tour#customizing-gh-actions-workflows-in-forked-repositories)

That we use this rather unusual mechanism is, of course, mostly a workaround for idiosyncracies in our procedures regarding (1) how we use the Sage repository and (2) how we make releases (this is documented in https://doc.sagemath.org/html/en/developer/review.html#the-release-process; but some of it needs updating).

Some of the relevant facts about our procedures:

- Currently, only the Release Manager (Volker) pushes to the "develop" branch of our repository, and does so only when development releases (beta/rc/stable) are tagged. 

- Betas are tagged on a roughly weekly cadence (sometimes slower), rcs sometimes on a faster cadence.

- Outside of the release candidate stage, the priority labels including "blocker" have no influence on when a PR is merged.

- Within the release candidate stage, the Release Manager will only merge (positively reviewed) PRs with the "blocker" prority.

- When a merge conflict arises, the Release Manager will set the PR to "needs work"; developers usually wait for the next development release to discover and resolve the merge conflict.

Why I reused the "blocker" priority for the "CI fixes" mechanism: 

- Outside of the release candidate stage, there was relatively little use of the "blocker" label, simply because it is of little consequence.

- Within the release candidate stage, developers who mark a PR as a "blocker" so that it be merged in the upcoming stable release need to know whether their blocker PR will be conflicting with other blockers (= candidates for merging in the next rc). Having the "blocker" label double as the "CI fixes" trigger takes care of this.

I'll note that some of this touches matters of governance of the project as well, a broader discussion of which would be timely.

[Dima Pasechnik]

A broader discussion on the governance is long overdue. We have a number of pending proposals to vote on,

but somehow nobody dares to break the filibuster from certain sides.

In particular, the question of allowing (allowing! allowing! - not requiring!) standard packages to be pip packages is pending.

We cannot get through a straightforward PR renaming install-requires.txt files to something more meaningful,

and standard, as the author keeps coming up with objections to the most reasonable choice, renaming them to

requirements.txt (format and semantics of install-requires.txt and requirements.txt - which is the standard name - are pretty much the same).

Some users positively review their own PRs, consisting of cherry-picked commits from PRs of other users they declare "disputed", as if it's an acceptable practice.

See https://github.com/sagemath/sage/pull/37482

Users get banned from even commenting on some PRs, without any notification. This has to stop, it's not the way to ensure any kind of normality - to the contrary, it's a sure way to hell.

E.g. my attempt to comment that the latest meson is 1.3.2 on https://github.com/sagemath/sage/pull/37319

results in "There was a problem submitting your review". WTF, really?

If someone wants to ban me, it has to be done directly, not in a such sneaky way.

I wanted to comment on https://github.com/sagemath/sage/pull/37482 that it has to be reviewed in a normal way, but no, I can't, just in the same way as on #37319

Dima

[Kwankyu Lee]

On Tuesday, February 27, 2024 at 2:43:18 AM UTC+9 Vincent Delecroix wrote:

In that case, let me do a proposal.

Introduce a new label distinct from "blocker" for

usage 2: PRs that should be merged temporarily before CI tests run

I meant by "merged temporarily" the "CI fixes" in Matthias' explanation:  

• Within the release candidate stage, developers who mark a PR as a "blocker" so that it be merged in the upcoming stable release need to know whether their blocker PR will be conflicting with other blockers (= candidates for merging in the next rc). Having the "blocker" label double as the "CI fixes" trigger takes care of this.

So blocker PRs get the chance to be tested together before the release by the "CI fixes" mechanism. Thus "usage 1" and "usage 2" are connected. Having distinct labels for them does not reflect the connection.

I propose (as this discussion is a place to give proposals :-) to give "the chance to be tested together" only to blocker PRs with "positive review". This slightly separates "usage 1" and "usage 2". This proposal was suggested when the "CI fixes" mechanism was introduced, and can be activated easily.

 

 

 

[John H Palmieri]

I think that usage (1) is the correct use of "blocker," and usage (3) is not. Usage (2) should have a new name, as Vincent proposes. Failing that, this new use of "blocker" must be documented in https://doc.sagemath.org/html/en/developer/review.html.

[John H Palmieri]

(and Tobias also proposed in https://github.com/sagemath/sage/issues/37428)

[Matthias Koeppe]

On Monday, February 26, 2024 at 1:59:11 PM UTC-8 Matthias Koeppe wrote:

(2) how we make releases (this is documented in https://doc.sagemath.org/html/en/developer/review.html#the-release-process; but some of it needs updating).

I've opened https://github.com/sagemath/sage/pull/37487 with an update of the description of the release process. 

[David Roe]

On Mon, Feb 26, 2024 at 8:06 PM John H Palmieri <jhpalm...@gmail.com> wrote:

I think that usage (1) is the correct use of "blocker," and usage (3) is not. Usage (2) should have a new name, as Vincent proposes. Failing that, this new use of "blocker" must be documented in https://doc.sagemath.org/html/en/developer/review.html.

I also agree that usage (2) should get a new name.  How about "CIFix?"

David

On Monday, February 26, 2024 at 4:21:58 PM UTC-8 Kwankyu Lee wrote:

On Tuesday, February 27, 2024 at 2:43:18 AM UTC+9 Vincent Delecroix wrote:

In that case, let me do a proposal.

Introduce a new label distinct from "blocker" for

usage 2: PRs that should be merged temporarily before CI tests run

I meant by "merged temporarily" the "CI fixes" in Matthias' explanation:  

• Within the release candidate stage, developers who mark a PR as a "blocker" so that it be merged in the upcoming stable release need to know whether their blocker PR will be conflicting with other blockers (= candidates for merging in the next rc). Having the "blocker" label double as the "CI fixes" trigger takes care of this.

So blocker PRs get the chance to be tested together before the release by the "CI fixes" mechanism. Thus "usage 1" and "usage 2" are connected. Having distinct labels for them does not reflect the connection.

I propose (as this discussion is a place to give proposals :-) to give "the chance to be tested together" only to blocker PRs with "positive review". This slightly separates "usage 1" and "usage 2". This proposal was suggested when the "CI fixes" mechanism was introduced, and can be activated easily.

 

 

 

--

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/001204a7-53fe-43ec-8be6-d2dbdd15b69dn%40googlegroups.com.

[Travis Scrimshaw]

For me, big +1 on (mostly) decoupling (2) from the rest. I think Kwankyu's suggestion for blockers with positive review being added to all CIs is a good way to do this. I don't see much utility in doing this at any other stage.

Best,

Travis