-
Notifications
You must be signed in to change notification settings - Fork 22.6k
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
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
b06fa20
chore(ci): Update labeler following learn move, make matching coarser
bsmth b328ad5
chore(ci): Update labeler following learn move, make matching coarser
bsmth efc8181
chore(ci): Update labeler following learn move, make matching coarser
bsmth 2ece8d5
Merge branch 'main' into labeler-rename
bsmth ff4610e
chore: restore deleted directory configs as comment
bsmth c176b51
chore: Remove learn from codeowners
bsmth cd02a23
Merge branch 'main' into labeler-rename
bsmth c693c16
Merge branch 'main' into labeler-rename
bsmth File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, I'll take a look at doing this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in ff4610e
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.