Skip to content

Labels converted from Trac Type, Component, Priority, Resolution, Status, special Milestones, some Keywords #8

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mkoeppe opened this issue Sep 26, 2022 · 23 comments

Comments

@mkoeppe
Copy link

mkoeppe commented Sep 26, 2022

as proposed in https://github.com/sagemath/sage/wiki/migration-from-trac-to-Git**b#proposed-workflow-on-github-with-transition-guide-from-trac

Type ("defect", "enhancement", "task") is mapped to a "Label" with prefix t:, e.g., t: bug
Component ("basic arithmetic", "linear algebra", "geometry", ...) are mapped to "Labels" with prefix c:
Priority ("major"/"minor"/"critical") is mapped to "Labels" with prefix p:

@mkoeppe mkoeppe changed the title Prefixes t: c: p: for labels converted from Trac Type, Component, Pritority Prefixes t: c: p: for labels converted from Trac Type, Component, Priority Sep 26, 2022
@tobiasdiez
Copy link

The labels can also be renamed after the migration

@soehms
Copy link
Member

soehms commented Oct 17, 2022

The labels can also be renamed after the migration

Why do you want to rename them? Because the name is unsatisfactory or because you want to change the meaning? In both cases we also need to adapt this to the migrated tickets (to keep them visible there). In the first case, it would be more efficient to find a satisfactory name before migrating. In the second case, there could be confusion regarding the migrated tickets.

I think the main problem we have here is that the GitHub labels are not a suitable substitute for select lists. If these labels are in place, they can also be used for new issues (not just migrated tickets). Then there may be issues marked as p:blocker and p:trivial at the same time. If mapping the Trac select lists to labels is the only option, we need GitHub actions (triggered by on: issues: types: [labeled, unlabeled]) to ensure that only one label of each group (p:.... , t:... ...) is set.

As for naming, I would prefer if they were all prefixed with trac (for example trac_p_major) so that users who have never worked with Trac can see where they came from. Is there a special reason to choose a comma as a separator?

@tobiasdiez
Copy link

What I meant was that the prefixes don't necessarily have to be added as part of the migration, i.e. its fine to migrate tickets and label them with "bug" , and then after the migration rename the label to "t: bug" without any issues (and this then automatically applies to all issues). If its however easy to prefix them in the script, then there is of course also no harm in doing this now.

we need GitHub actions (triggered by on: issues: types: [labeled, unlabeled]) to ensure that only one label of each group

That only makes sense for some groups. For example, you can easily imagine an issue that is a bug and needs changes to the docs (t: bug + t: docs).

The idea was to keep using these labels also for new issues/pull-requests, right?

@soehms
Copy link
Member

soehms commented Oct 17, 2022

What I meant was that the prefixes don't necessarily have to be added as part of the migration, i.e. its fine to migrate tickets and label them with "bug" , and then after the migration rename the label to "t: bug" without any issues (and this then automatically applies to all issues). If its however easy to prefix them in the script, then there is of course also no harm in doing this now.

I see! Sorry for misunderstanding!

That only makes sense for some groups.

Sure! Groups that come from select list and radio buttons via the migration.

The idea was to keep using these labels also for new issues/pull-requests, right?

Independent whether that is the idea or not, if they are there they can be used. Or can we block them after the migration?

@tobiasdiez
Copy link

Or can we block them after the migration?

Nope, as of now, you can only delete labels (which then also removes them from the issues). A workaround would be to rename them to something that sort them last and add in the label description that they shouldn't be used.

@soehms
Copy link
Member

soehms commented Oct 17, 2022

Or can we block them after the migration?

Nope, as of now, you can only delete labels (which then also removes them from the issues). A workaround would be to rename them to something that sort them last and add in the label description that they shouldn't be used.

Maybe changing habits will be easier if we keep them (see my recent post on sage-devel). But when we do that, we should eliminate sources of confusion.

@mkoeppe
Copy link
Author

mkoeppe commented Dec 24, 2022

Also we should not create "PLEASE CHANGE" labels - https://github.com/sagemath/trac_to_gh/blob/main/Issue%2018792.md

@mkoeppe
Copy link
Author

mkoeppe commented Dec 24, 2022

In a8ace24, I introduce a "component: " prefix (instead of the cryptic "c: ") and reduce noise by suppressing labels for the default priority and for ticket types other than "bug".

@mkoeppe mkoeppe closed this as completed Dec 24, 2022
@tobiasdiez
Copy link

I would still add types for enhancement/feature request and questions. You can always remove labels later, but it's going to be hard to add the correct labels after the migration. Also it's handy to e.g. filter out questions (which me might want to convert to Discussions anyway).

@tobiasdiez
Copy link

The ticket status (needs review etc) should also be mapped to a label.

@mkoeppe mkoeppe reopened this Dec 28, 2022
@mkoeppe
Copy link
Author

mkoeppe commented Dec 28, 2022

Not sure about "needs review" because that is just the status of a non-draft, non-approved PR. (See #57 "create PRs instead of Issues for closed tickets with branches")

@mkoeppe
Copy link
Author

mkoeppe commented Dec 28, 2022

But I agree that we should make an effort to map to the default labels defined by Github:

  • "duplicate" (map from Resolution: duplicate)
  • "enhancement" (map from Type: enhancement)
  • "good first issue" (map from keyword: beginner)
  • "help wanted"
  • "invalid" (map from Resolution: invalid),
  • "question" (map from Status: needs_info)
  • "wontfix" (map from Resolution: wontfix)

@mkoeppe
Copy link
Author

mkoeppe commented Dec 28, 2022

We may also want to get rid of the special milestones and map them to labels instead:

  • "sage-duplicate/invalid/wontfix" milestone: map it to label "invalid" unless Resolution is set to something more specific
  • "sage-feature" map to label "feature"
  • "sage-pending" map to label "pending"
  • "sage-wishlist" map to label "wishlist"

@mkoeppe
Copy link
Author

mkoeppe commented Dec 28, 2022

In the code, we can change keywords_to_labels from boolean to a dictionary - some keywords to be mapped to labels (for "beginner"). Done in e46ee3d

@mkoeppe
Copy link
Author

mkoeppe commented Dec 28, 2022

Resolutions are now mapped to labels (done in 40f262d)

@mkoeppe mkoeppe changed the title Prefixes t: c: p: for labels converted from Trac Type, Component, Priority Labels converted from Trac Type, Component, Priority, Resolution, Status, special Milestones, some Keywords Dec 29, 2022
@mkoeppe
Copy link
Author

mkoeppe commented Dec 29, 2022

Some early tickets (for example https://trac.sagemath.org/ticket/3304) have keywords "editor_mabshoff", "editor_wstein", etc. @williamstein
Should these be mapped to something? Is this the same as a reviewer?

@tobiasdiez
Copy link

Not sure about "needs review" because that is just the status of a non-draft, non-approved PR. (See #57 "create PRs instead of Issues for closed tickets with branches")

In theory yes, but in practice you often have a non-draft, non-approved PR that needs (minor) revision by the opener. Github sadly doesn't have a good search function for "has a review requiring changes" and thus many repos use a "needs review" and "needs work" label to indicate what is required to move this PR forward. As a project maintainer this is usually important, as you can easily find PRs that need your active review and, on the other hand, from time to time you can go though the list of "needs work" and remind people.

@tobiasdiez
Copy link

We may also want to get rid of the special milestones and map them to labels instead:

* [ ]  "sage-feature" map to ???? 

t: feature or t: enhancement?

* [ ]  "sage-pending" map to ????

* [ ]  "sage-wishlist" map to ???

Looks like these are better handled as Projects instead of milestones/labels.

@mkoeppe
Copy link
Author

mkoeppe commented Dec 30, 2022

For now (in df64414) I'm just mapping sage-{feature,pending,wishlist} to labels, just to preserve the information. Better than as a ticket comment.

I agree that Projects is something that we want to look into, but I'm not going to attempt that in the migration script.

@mkoeppe
Copy link
Author

mkoeppe commented Dec 30, 2022

Github sadly doesn't have a good search function for "has a review requiring changes" and thus many repos use a "needs review" and "needs work" label to indicate what is required to move this PR forward.

OK, good points there. I'll map them to labels. Preparation for this is in 9c0c175. Probably best to refactor label handling next.

@mkoeppe
Copy link
Author

mkoeppe commented Dec 31, 2022

Status is mapped to labels now as of 0f5c82d.

@mkoeppe
Copy link
Author

mkoeppe commented Dec 31, 2022

I think this is in good shape now.

@mkoeppe mkoeppe closed this as completed Dec 31, 2022
@mkoeppe mkoeppe added this to the Migration archive milestone Jan 1, 2023
@soehms
Copy link
Member

soehms commented Jan 13, 2023

Github sadly doesn't have a good search function for "has a review requiring changes" and thus many repos use a "needs review" and "needs work" label to indicate what is required to move this PR forward.

OK, good points there. I'll map them to labels. Preparation for this is in 9c0c175. Probably best to refactor label handling next.

It's a good idea to have needs_review or needs_work as labels. On the other hand, since this is redundant information, we need to sync these labels with the GitHub states. Otherwise they will immediately run out of reliability.

I think we can use triggers like this for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants