normalize issue labels with other dart-lang repos

[devoncarew]

Hi - we're working to normalize issue labels across the dart-lang repos. This is particularly topical for this repo, as I imagine the issues here will be transferred into the dart-lang/sdk repo. Here's the current diff of this repo's labels vs the canonical dart-lang set (hosted at dart-lang/.github):

  (rename) [ux]: rename => type-ux, color => #fff9e3, description => "A user experience or user interface r..."
  (update) [autosubmit]: color => #1D76DB
  (update) [contributions-welcome]: color => #C2E0C6, description => "Contributions welcome to help resolve..."
  (update) [good first issue]: color => #C2E0C6, description => "A good starting issue for contributor..."
  (update) [needs-info]: color => #444444, description => "Additional information needed from th..."
  (update) [P0]: description => "A serious issue requiring immediate r..."
  (update) [P1]: color => #f9c0b4, description => "A high priority bug; for example, a s..."
  (update) [P2]: description => "A bug or feature request we're likely..."
  (update) [P3]: color => #FEF2C0, description => "A lower priority bug or feature request"
  (update) [type-bug]: color => #fff9e3, description => "Incorrect behavior (everything from a..."
  (update) [type-code-health]: color => #fff9e3, description => "Internal changes to our tools and wor..."
  (update) [type-documentation]: color => #fff9e3, description => "A request to add or improve documenta..."
  (update) [type-enhancement]: color => #fff9e3, description => "A request for a change that isn't a bug"
  (update) [type-question]: color => #fff9e3, description => "A question about expected behavior or..."
  (add) [closed-as-intended]: #686868 "Closed as the reported issue is expected behavior"
  (add) [closed-cannot-reproduce]: #686868 "Closed as we were unable to reproduce the reported issue"
  (add) [closed-duplicate]: #686868 "Closed in favor of an existing report"
  (add) [closed-invalid]: #686868 "Closed as we don't believe the reported issue is generally actionable"
  (add) [closed-not-planned]: #686868 "Closed as we don't intend to take action on the reported issue"
  (add) [closed-obsolete]: #686868 "Closed as the reported issue is no longer relevant"
  (add) [closed-stale]: #686868 "Closed as the issue or PR is assumed stale."
  (add) [status-blocked]: #444444 "Blocked from making progress by another (referenced) issue"
  (add) [type-infra]: #fff9e3 "A repository infrastructure change or enhancement"
  (consistency) [dependency: analyzer]: avoid spaces; use lowercase-dashes to join words
  (consistency) [effective dart]: avoid spaces; use lowercase-dashes to join words
  (consistency) [false negative]: avoid spaces; use lowercase-dashes to join words
  (consistency) [false positive]: avoid spaces; use lowercase-dashes to join words
  (consistency) [lint proposal]: avoid spaces; use lowercase-dashes to join words
  (consistency) [lint request]: avoid spaces; use lowercase-dashes to join words
  (consistency) [needs internal testing]: avoid spaces; use lowercase-dashes to join words
  (consistency) [new language feature]: avoid spaces; use lowercase-dashes to join words
  (consistency) [NNBD]: avoid upper case; use lowercase names
  (consistency) [process]: avoid single word labels (prefer using a category prefix)
  (consistency) [pub]: avoid single word labels (prefer using a category prefix)
  (consistency) [set-flutter (dev)]: avoid spaces; use lowercase-dashes to join words
  (consistency) [set-internal (available)]: avoid spaces; use lowercase-dashes to join words
  (consistency) [status: accepted]: avoid spaces; use lowercase-dashes to join words
  (consistency) [status: closed]: avoid spaces; use lowercase-dashes to join words
  (consistency) [status: in progress]: avoid spaces; use lowercase-dashes to join words
  (consistency) [status: pending]: avoid spaces; use lowercase-dashes to join words
  (consistency) [style]: avoid single word labels (prefer using a category prefix)

Given the number of issues in this repo I think we should prep the repo with a bunch of manual label renames / adds / updates before we run the label normalization tool here; cc @pq.

[pq]

Taking a look at this today. My initial focus will be the consistency violations thinking that the we can auto-resolve the other ones.

• [x] (consistency) [dependency: analyzer]: avoid spaces; use lowercase-dashes to join words
• [x] (consistency) [effective dart]: avoid spaces; use lowercase-dashes to join words
• [x] (consistency) [false negative]: avoid spaces; use lowercase-dashes to join words
• [x] (consistency) [false positive]: avoid spaces; use lowercase-dashes to join words
• [x] (consistency) [lint proposal]: avoid spaces; use lowercase-dashes to join words
• [x] (consistency) [lint request]: avoid spaces; use lowercase-dashes to join words
• [x] (consistency) [needs internal testing]: avoid spaces; use lowercase-dashes to join words
• [x] (consistency) [new language feature]: avoid spaces; use lowercase-dashes to join words
• [x] (consistency) [NNBD]: avoid upper case; use lowercase names
• [x] (consistency) [process]: avoid single word labels (prefer using a category prefix)
• [x] (consistency) [pub]: avoid single word labels (prefer using a category prefix)
• [x] (consistency) [set-flutter (dev)]: avoid spaces; use lowercase-dashes to join words
• [x] (consistency) [set-internal (available)]: avoid spaces; use lowercase-dashes to join words
• [x] (consistency) [status: accepted]: avoid spaces; use lowercase-dashes to join words
• [x] (consistency) [status: closed]: avoid spaces; use lowercase-dashes to join words
• [x] (consistency) [status: in progress]: avoid spaces; use lowercase-dashes to join words
• [x] (consistency) [status: pending]: avoid spaces; use lowercase-dashes to join words
• [x] (consistency) [style]: avoid single word labels (prefer using a category prefix)

[pq]

@devoncarew: besides consistency, we want to scope these labels appropriately too right? That is, false-positive might become something like linter-false-positive (assuming we have an area-linter).

Since it's a simple prefixing, I guess we can bulk do that just before the move? Unless you'd prefer it sooner?

[pq]

Also, does your tool do updates automatically or, for example, should I go through and manually edit colors and so on?

[devoncarew]

That is, false-positive might become something like linter-false-positive (assuming we have an area-linter).

Yup - we'd want to prefix to help scope this in the larger repo.

The tool will do color + description updates for the labels it knows about. It's easy enough to make a pass through after the tool runs.

[pq]

Yup - we'd want to prefix to help scope this in the larger repo.

Cool.

My thinking there is a new area-linter and a bulk application of linter to the labels here. (I can see a few tweaks we could make to a few individual labels but this is good enough for most of them IMO.)

@bwilkerson, @srawlins: any feelings?

[bwilkerson]

Interesting question. I don't have a strong opinion, but I will note that at some point in the past (I don't know when), someone (I don't know who) decided that there should be an area-analyzer, but that we wouldn't create an area-analysis-server.

Inventing a rationale after the fact (which might or might not be the original rationale), I'd guess that it was done because users shouldn't need to know how the code is divided into packages but should be focused more on the tool that they're reporting against. That same rationale could apply here, arguing against having an area-linter label.

That said, I don't know whether users can add labels, and even if they can I don't know how often users use labels for searching, etc. In other words, I don't know what the impact of this decision is on the community. I only know that it ought to be a consideration in the decision.

If we do decide to have an area-linter (whether because it's better for the community at large or for our own management of issues), then it might be worthwhile to also add an area-analysis-server for the same reason (although some of the labels would be really long if we did).

[srawlins]

I prefer area-analyzer, and analyzer-linter, and analyzer-linter-false-positive.

[parlough]

users shouldn't need to know how the code is divided into packages but should be focused more on the tool that they're reporting against.

I think this is key. Even though users generally can't add labels, they may still reference or refer to labels. I think the linter should be more and more of an implementation detail of the analyzer rather than a standalone entity. In that vein, it's probably best under area-analyzer :)

[pq]

Thanks!

I guess I should have weighed in w/ my own preference too!

Two strikes I see against analyzer-linter are

1. the length of labels being kind of ungainly (analyzer-linter-rule-set-core oof) but more significantly
2. the added time it takes to find the right label when doing triage (typing analyzer-linter- is 9 characters longer than linter- ...)

The second one, for me is a doozy.

[pq]

I think the linter should be more and more of an implementation detail of the analyzer rather than a standalone entity.

Hmmmm. Maybe?

What's funny is that today almost the opposite is true. The linter lives at the top level of your analysis options and is mostly the thing you configure.

On the other hand you do run the analyzer. So we're not consistent. 😬

[pq]

On the other hand you do run the analyzer.

(Unless you're in an IDE and then it may just seem like magic.)

[srawlins]

"Linter" being a noun does imply it is a separate tool, but it is not any longer. I think we can move the linter code into pkg/analyzer/lib/src/linter, and then, yes, everything is analyzer, and what is interesting is "lint rule" vs "warning" vs "error".

I am also in favor of moving the lint rule configuration, in analysis_options.yaml, out of the top-level linter key, into analyzer, and combining lint rule configuration, error/warning configuration, and strict rule enablement into fewer sections. Something like:

analyzer:
  enable:
    - directives_ordering
    - prefer_const_constructors
    - strict-casts
    - strict-raw-types
  severity-overrides:
    unused_element: ignore
    todo: ignore

[srawlins]

Or if we allow configuring rules per directory:

analyzer:
  rule-configurations:
    unused_element:
      severity: ignore
    camel_case_names:
      exclude: test/

[parlough]

what is interesting is "lint rule" vs "warning" vs "error".

I agree with everything here and this is why. Linter rules are essentially an off-by-default diagnostic that defaults to "info", but it shouldn't (need to) be seen as different from the other diagnostics. Hence why we want to incorporate their new docs into https://dart.dev/tools/diagnostic-messages.

Sorry this got a bit out of scope of the issue, but it does seem relevant in this decision. I think either works for now, as labels can easily be adjusted/reworked :)

[pq]

@srawlins I 💯 hear you on the desire to re-work analysis options. It's clumsy and full of historical accidents.

Since it would be so ecosystem impacting we even talked about doing it for Dart 3.0 IIRC but the costs kind of outweighed the benefits.

Happy to revisit!

[pq]

Sorry this got a bit out of scope of the issue

Not at all! I think it's a good conversation to have.

the added time it takes to find the right label when doing triage

Regarding this, I wonder if I'm missing some hacks. Are there things we could do to make triaging easier? When we migrate to the SDK we'll lose our automated labelling. (At least temporarily -- engprod seemed open to supporting it down the road.)

[parlough]

Regarding this, I wonder if I'm missing some hacks. Are there things we could do to make triaging easier? When we migrate to the SDK we'll lose our automated labelling. (At least temporarily -- engprod seemed open to supporting it down the road.)

You can use GitHub issue templates to automatically apply area labels.

That template could also include other information only relevant for the analyzer/linter, providing better starting info.

There might be some other fancy stuff you can do with the templates to apply more specific labels, not sure. They keep expanding the functionality.

[srawlins]

When we migrate to the SDK we'll lose our automated labelling. (At least temporarily -- engprod seemed open to supporting it down the road.)

I'm curious about this. In the linter repo, it was just a GitHub Action that labeled the issues, right? nothing that needs supporting?

Some of these labels do get unwieldy; I think analyzer-false-positive and analyzer-false-negative would be fine. The status ones though ☹️ . analyzer-lint-proposal-status-accepted? 😦

[pq]

You can use GitHub issue templates to automatically apply area labels.

The issue here is not crowding the top-level with linter/analyzer-specific templates.

I'm curious about this. In the linter repo, it was just a GitHub Action that labeled the issues, right? nothing that needs supporting?

Correct. There just aren't any actions like this for the SDK issue repo. They seemed open to the idea (and it could be broadened) but it'd need to be evaluated. I plan to chat with folks at the summit about just this!

[devoncarew]

It looks like there's been some good discussion on this issue. Just to revisit what I filed in the initial comment, that work is now complete (similar labels renamed to our std labels, remaining std labels added, and the format of some existing labels tweaked to be in kebab-case form).

In terms of what additional label to apply to these issues when moving them into the sdk repo, that's just up to whatever you think would be most helpful with initial triage and ongoing searching / categorization (analyzer-linter? area-analyzer and analyzer-linter?). I will point out that we do get a large volume of linter related issues.

[pq]

After a bunch of back and forth, I think we've settled on using the existing area-analyzer in the SDK and introducing new sub-area labels for the linter issues. We'll be careful to make sure the labeling is "reversable" so we can move to an area-linter approach if this gets unmanagable.

@devoncarew: if I update the labels, will you do the issue transferring? Or is there a plan for that?

[devoncarew]

There's a tool we wrote you can use (and there may be other hosted ones which work as well).

You can clone and run the tool from https://github.com/dart-lang/ecosystem/tree/main/pkgs/repo_manage.

You then run dart bin/report.dart transfer-issues <options>, the the args being:

Bulk transfer issues from one repo to another.

Usage: report transfer-issues [arguments] --source-repo repo-org/old-repo-name --target-repo repo-org/new-repo-name --add-label pkg:old-repo-name
-h, --help                                            Print this usage information.
    --apply-changes                                   WARNING: This will transfer the issues. Please preview the changes first by running without '--apply-changes'.
    --issues=<1,2,3>                                  Specifiy the numbers of specific issues to transfer, otherwise transfers all.
    --source-repo=<repo-org/repo-name> (mandatory)    The source repository for the issues to be moved from.
    --target-repo=<repo-org/repo-name> (mandatory)    The target repository name where the issues will be moved to.
    --add-label=<pkg:foo>                             Add a label to all transferred issues.

Given the number of issues here, you may want to run first w/o --apply-changes, or even locally modify the source for the tool to only try and transfer smaller batches at a time.

[devoncarew]

That tool can optionally add a new label to each transferred issue.