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

Aggregate labels from merged pull requests #56

Closed
wants to merge 4 commits into from

Conversation

ohbarye
Copy link
Collaborator

@ohbarye ohbarye commented Sep 22, 2020

@onk @motemen

Issue

Closes #54

Changes

As described in #54, I added aggregate_labels feature.

  • Add pr-release.aggregate_labels config (or GIT_RELEASE_PR_AGGREGATE_LABELS).
    • It accepts boolean-ish value: "true" or anything
    • Only when it is "true", git-release-pr aggregates labels from merged pull requests.
  • What if pr-release.labels is given at the same time?
    • Merge aggregated labels and pr-release.labels setting.
      • pr-release.labels=foo,bar and aggregated labels are baz, then labels would be ["foo", "bar", "baz"].

Testing

That worked well on my own

$ GIT_PR_RELEASE_BRANCH_STAGING=release GIT_PR_RELEASE_LABELS=release GIT_PR_RELEASE_AGGREGATE_LABELS=true bundle exec git-pr-release
Merged PRs have labels Created PR has labels
image image

Review Points

  • Is aggregate_labels an appropriate name?
    • ideas: reuse_labels, summarize_labels... etc.
  • Should aggregate_labels be given as a command line arg, not environment variable or git config?
    • As like --dry-run, --no-fecth, aggregate_labels is boolean-ish value. So I wonder if it should be. (It depends on this gem's command line design.)

:assignees: []
:requested_reviewers: []
:requested_teams: []
:labels: []
:milestone:
:labels:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

📝 This example comes from the ofiicial API reference.

https://docs.github.com/en/rest/reference/pulls#get-a-pull-request--code-samples

@@ -75,7 +76,8 @@ def configure
say "Production branch: #{production_branch}", :debug
say "Staging branch: #{staging_branch}", :debug
say "Template path: #{template_path}", :debug
say "Labels #{labels}", :debug
say "Labels: #{labels}#{aggregate_labels ? ' + labels on merged PRs' : ''}", :debug
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wanted to show merged labels but it couldn't determine all labels to be added here.
Because it calls fetch_merged_prs after configure.


Due to the same reason, I was compelled to add a destructive substitution to @labels in fetch_merged_prs.

@labels = (labels + merged_prs_labels).uniq

@ohbarye ohbarye marked this pull request as ready for review September 22, 2020 06:02
@ohbarye ohbarye changed the title Aggregate labels from prs [DO NOT MERGE] Aggregate labels from prs Sep 22, 2020
```
$ git config pr-release.aggregate_labels
error: invalid key: pr-release.aggregate_labels
```
@ohbarye ohbarye changed the title [DO NOT MERGE] Aggregate labels from prs Aggregate labels from merged pull requests Sep 22, 2020
@ohbarye ohbarye requested a review from onk September 24, 2020 03:47
@ohbarye
Copy link
Collaborator Author

ohbarye commented Sep 15, 2023

This change got unnecessary for me, so I'm closing.

@ohbarye ohbarye closed this Sep 15, 2023
@ohbarye ohbarye deleted the aggregate-labels-from-prs branch September 15, 2023 09:27
@masaru-iritani
Copy link

@ohbarye I believe this feature is exactly what my team needs. Would it be possible for you to reopen this pull request, or would you prefer that I take over the work?

@ohbarye
Copy link
Collaborator Author

ohbarye commented Feb 10, 2024

@masaru-iritani Sure. Please go ahead as you like!

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

Successfully merging this pull request may close these issues.

[Proposal] Aggregate labels from pull requests
2 participants