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

auto label new PRs with type::feature and type::docs #943

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

e3b0c442
Copy link
Contributor

No description provided.

@e3b0c442 e3b0c442 requested a review from a team as a code owner February 22, 2023 17:09
@replicated-ci replicated-ci added type::docs Improvements or additions to documentation type::feature New feature or request labels Feb 22, 2023
@e3b0c442 e3b0c442 added type::chore and removed type::feature New feature or request type::docs Improvements or additions to documentation labels Feb 22, 2023
@github-actions
Copy link

@github-actions github-actions bot temporarily deployed to preview February 22, 2023 17:14 Inactive
github-token: ${{ secrets.DOCS_GH_PAT }}
script: |
const labels = ['type::feature', 'type::docs']
github.rest.issues.addLabels({
Copy link
Member

Choose a reason for hiding this comment

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

Thanks @e3b0c442. Not sure I fully understood it though, would you mind clarifying it a bit? Is it going to add the same labels to all open issues ? Ah, this is a little bit confusing, seems like we are adding labels to the opened pull requests (on.pull_request.types: ["opened"]) but at the same time the function seems to be adding them to the issues (github.rest.issues.addLabels) instead. Are we adding or making sure the labels exist on open issues every time a pull request is opened ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey, no worries.

The on clause determines which events trigger the workflow, so in this case, it only runs when pull requests are opened.

In GitHub, pull requests are just issues with a special purpose, so we call the issue label API to add a label, with the issue (pull request) number coming from the context.

So yes, this workflow adds those labels only to pull requests, and only when they are initially opened; it will not run again for any PR after being opened.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @e3b0c442 , that clarifies everything. Is this something we really want to do ? I mean, I bet we are going to start seeing PRs labeled as "feature" when they are "bugs" as less and less people stop even noticing labels ("green means all good").

Copy link
Contributor

Choose a reason for hiding this comment

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

To @ricardomaraschini point, I'd say we should at a minimum default to adding type::docs(since this primarily our kURL docs site) and have the author of the PR consciously determine whether it's type::featuretype::choretype::bug

Copy link
Contributor Author

@e3b0c442 e3b0c442 Feb 23, 2023

Choose a reason for hiding this comment

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

@paigecalvert needs to chime in here then. The ask was to reduce friction on the docs folks needing to manually add the labels. If 80% of the PRs to this repo are docs-only, then I think that makes sense. If the ratio is lower, we need to reconsider and come to an agreement between the two teams. We have already established that of the docs PRs, we hit that 80% threshold of feature/other.

There are some things that could be done if there's a clear delineation in the repo between docs and non-docs PRs as well; I can filter this workflow to a subset of paths in the repo, or we can look at fully splitting the repos so the teams can set their own policies.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the ping @e3b0c442. Yes, the ask here was to automatically apply these labels because:

There is a requirement that all PRs have to use a primary label, but the Docs team does not use the categories of "feature", "bug" or "chore" to classify the types of PRs in this repo. Those labels are relevant to code changes, but they don't really translate very well to how we would want to categorize updates to the docs. So, we figured, rather than having to manually add "feature" on every PR in order to just get the test to pass, we would ask to have it added automatically to save ourselves the extra step.

THAT SAID, if this creates issues for other teams, or if it could mess up the data for others, then we certainly would not want to do it. I know that Dalia also wants the docs repos to use the same labels as the dev repos, so perhaps it's better for me to make sure that she is bought in before we merge.

I do apologize for the confusion--we did not anticipate the back and forth this would bring about.

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

Successfully merging this pull request may close these issues.

5 participants