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

Use release-drafter/release-drafter GitHub Action to label our PRs #4868

Merged
merged 9 commits into from Nov 15, 2023

Conversation

aloisklink
Copy link
Member

@aloisklink aloisklink commented Sep 24, 2023

📑 Summary

Replace the TimonVS/pr-labeler-action with release-drafter/release-drafter (previously known as toolmantim/release-drafter, which we already use for drafting releases) as it has an autolabeler option that can autolabel PRs for us.

This should fix labeling PRs from forks, see TimonVS/pr-labeler-action#25 (this is why some PRs keep having the feature/fix/chore labels).

📏 Design Decisions

I've kept the .github/pr-labeler.yml configuration file, so that links to it from the https://mermaid.js.org/ website continue to work.

I've also kept everything in the same .github/workflows/pr-labeler.yml GitHub Actions workflow to make the git diff easier to review, and to keep the GitHub Actions permissions the same.

While I was here, I also set the GitHub Action permissions so that they have the minimum require permissions needed to do anything.

📋 Tasks

Make sure you

  • 📖 have read the contribution guidelines
  • 💻 have added necessary unit/e2e tests.
    • This PR should be labeled with the Type: Other label if everything works as expected. It looks like the auto-labeler will only work once everything is merged, since it looks at the pr-labeler.yml file of the develop branch.
  • 📓 have added documentation. Make sure MERMAID_RELEASE_VERSION is used for all new features.
    • Existing documentation links to pr-labeler.yml will continue to work, e.g.
      You can always check current [configuration of labelling and branch prefixes](https://github.com/mermaid-js/mermaid/blob/develop/.github/pr-labeler.yml)
  • 🔖 targeted develop branch

The pr-labeler.yml GitHub Action will fail in this branch, since it

Limit the `GITHUB_TOKEN` permissions for `toolmantim/release-drafter`
to the minimum required permissions.
Limit the `GITHUB_TOKEN` permissions for `TimonVS/pr-labeler-action`
to the minimum required permissions.
`branch` is not a valid configuration option for release-drafter,
see
https://github.com/release-drafter/release-drafter#configuration-options

There's is a similar [`references` option][1], but it does nothing when
using GitHub Actions (it's only there for GitHub apps).

There's also `commitish`, but it defaults to the target/branch the
GitHub Action job runs on, so we don't need to set that.

[1]: https://github.com/release-drafter/release-drafter#references
Using `pull_request_target` is pretty dangerous, since it heavily
increases the risk of malicious PRs getting access to the mermaid-js
repo.

What we're doing currently is safe, but we should add a warning
message just to ensure that we're very careful when we make changes.

See: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target
See: https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
Change the formatting of .github/pr-labeler.yml to make `git diff`'s
easier to understand in a future commit.
Replace the `TimonVS/pr-labeler-action` with
`release-drafter/release-drafter` as it has an [`autolabeler`][1]
option that can autolabel PRs for us.

This should fix labeling PRs from forks,
see TimonVS/pr-labeler-action#25.

I've kept the `.github/pr-labeler.yml` configuration file, so that
links to it from the https://mermaid.js.org website continue to work.

I've also kept everything in the same
`.github/workflows/pr-labeler.yml` GitHub Actions workflow to make the
`git diff` easier to review, and to keep the GitHub Actions permissions
the same.

[1]: https://github.com/release-drafter/release-drafter/blob/ff929b5ceb21bf2646a216e916f9a8bb507d48a3/README.md#autolabeler
@netlify
Copy link

netlify bot commented Sep 24, 2023

Deploy Preview for mermaid-js ready!

Name Link
🔨 Latest commit 862d20c
🔍 Latest deploy log https://app.netlify.com/sites/mermaid-js/deploys/65542cf34048090008796cb9
😎 Deploy Preview https://deploy-preview-4868--mermaid-js.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@codecov
Copy link

codecov bot commented Sep 24, 2023

Codecov Report

Merging #4868 (99beeba) into develop (c56025e) will decrease coverage by 1.75%.
The diff coverage is n/a.

❗ Current head 99beeba differs from pull request most recent head 862d20c. Consider uploading reports for the commit 862d20c to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #4868      +/-   ##
===========================================
- Coverage    79.33%   77.58%   -1.75%     
===========================================
  Files          164      148      -16     
  Lines        13864    13025     -839     
  Branches       698      612      -86     
===========================================
- Hits         10999    10106     -893     
- Misses        2715     2765      +50     
- Partials       150      154       +4     
Flag Coverage Δ
e2e 82.15% <ø> (-3.09%) ⬇️
unit 43.67% <ø> (+0.76%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 46 files with indirect coverage changes

@aloisklink aloisklink added the Type: Other Not an enhancement or a bug label Sep 24, 2023
.github/workflows/pr-labeler.yml Outdated Show resolved Hide resolved
This value is unused, but it's required, so we have to add it.

Fixes: a1673d3
Copy link
Collaborator

@knsv knsv left a comment

Choose a reason for hiding this comment

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

Look good!

* develop: (164 commits)
  Update all patch dependencies
  Fix docs
  Update packages/mermaid/src/docs/community/questions-and-suggestions.md
  Update packages/mermaid/src/diagrams/class/classRenderer-v2.ts
  update edge ids
  draw top actors with lines  first followed by messages
  Bump GitHub workflow actions to latest versions
  Update docs
  Documentation: clarify sentence
  Fix lint
  Fix typo
  fix typo
  Add new Atlassian integrations
  chore(deps): update all patch dependencies
  Update demos/sequence.html
  chore: release v10.6.1
  fix
  fix
  fix: render the participants in same order as they are created
  fix(flow): fix invalid ellipseText regex
  ...
@sidharthv96 sidharthv96 merged commit e8ee5f5 into develop Nov 15, 2023
22 of 23 checks passed
@sidharthv96 sidharthv96 deleted the other/remove-pr-labeler-action branch November 15, 2023 02:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: DevOps Type: Other Not an enhancement or a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants