Skip to content
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

chore(ci): Labeler needs updating after Learn move #37294

Merged
merged 8 commits into from
Jan 28, 2025
Merged

Conversation

bsmth
Copy link
Member

@bsmth bsmth commented Dec 20, 2024

Description

The content under learn_web_development may change over the coming weeks and months. While these changes are happening, it may be a good idea to make the matching a little more coarse for issues and PRs.

Motivation

Issues and PRs do not get matched against the current main due to directory renaming.

Related issues and pull requests

@bsmth bsmth requested a review from a team as a code owner December 20, 2024 11:59
@github-actions github-actions bot added system [PR only] Infrastructure and configuration for the project size/m [PR only] 51-500 LoC changed labels Dec 20, 2024
@@ -19,27 +19,7 @@ Content:HTTP:
Content:JS:
- '\/docs\/Web\/JavaScript'
Content:Learn:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not familiar with the new IA but do we not have these subareas anymore? It would still be helpful for domain-specific tutorials to be assigned to specific triagers/reviewers.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I agree, I already started mapping these out in b328ad5 but I'm hesitant about it due to how much flux may happen here soon.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you keep the other ones as comments or empty arrays? I want us to eventually use the exactly same set of labels unless you plan to remove or add entire areas,

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep the [deleted] ones as comments or empty arrays

Good idea, I'll take a look at doing this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in ff4610e

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, is there a reason why the labels can't describe the IA at the time being? Unless the migration will be done in a week, it would still be nice to have fine-grained triaging for all the different topics, especially for directing them to different maintainers. If we have to update this file again after the move anyway, it won't cause extra work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of making the learn review assignments less granular; sure, it makes sense for domain experts to review the main MDN topic areas, but what is also important is the learn content having a consistent style, depth, and approach. This is something the coming updates will concentrate on quite a lot.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm focusing on the Django and Express docs. They are currently almost exclusively reviewed by Hamish and I doubt the state will change much even if we remove the granularity. Sure I'd be glad for all core docs to be reviewed by the same group of people, but extension docs are topical in their nature that others may not find comfortable in reviewing.

Also I'm more comfortable with the reviewer pool being shared within the entire learning area. But labels should be applied as granularly as possible so to aid in searching, such as for old issues/PRs or looking for duplicates.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we can merge this and get a follow-up for applying old labels? Currently we're missing Learn labels entirely, so I think a broader scope is okay until we have a solid idea what all subcategories should look like.

@bsmth bsmth requested a review from Josh-Cena January 20, 2025 15:28
@Josh-Cena
Copy link
Member

Could you update https://github.com/mdn/content/blob/main/.github/CODEOWNERS as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes:

  • /files/en-us/learn paths are removed
  • entries are alphabetical by path on file system

Copy link
Member

@Josh-Cena Josh-Cena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to leave an approval for you to merge

@bsmth
Copy link
Member Author

bsmth commented Jan 28, 2025

Thanks a lot, going to merge shortly

@bsmth bsmth merged commit e70e952 into mdn:main Jan 28, 2025
7 of 8 checks passed
@bsmth bsmth deleted the labeler-rename branch January 28, 2025 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/m [PR only] 51-500 LoC changed system [PR only] Infrastructure and configuration for the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants