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

fix: add workflow to notify docs triagers on relevant PRs #2660

Merged
merged 20 commits into from
Mar 16, 2024

Conversation

TRohit20
Copy link
Collaborator

Description
The PR introduces a new workflow to the website repo which runs every time a new PR is raised and checks if any .md files were modified in any manner, if yes, the workflow will auto-comment tagging docs-triagers for review and approval.

Related issue(s)
Resolves #2634

PS: Ofc, we would have to configure the secret(GH_Token) first to make this work.

Screenshot 2024-02-16 at 8 54 36 PM

Copy link

netlify bot commented Feb 16, 2024

Deploy Preview for asyncapi-website ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 54e16fb
🔍 Latest deploy log https://app.netlify.com/sites/asyncapi-website/deploys/65f58b6442d8900008fe6b6b
😎 Deploy Preview https://deploy-preview-2660--asyncapi-website.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.

@asyncapi-bot
Copy link
Contributor

asyncapi-bot commented Feb 16, 2024

⚡️ Lighthouse report for the changes in this PR:

Category Score
🔴 Performance 31
🟢 Accessibility 98
🟢 Best practices 92
🟢 SEO 100
🔴 PWA 33

Lighthouse ran on https://deploy-preview-2660--asyncapi-website.netlify.app/

@quetzalliwrites
Copy link
Member

thank you for this!

Copy link
Member

@akshatnema akshatnema left a comment

Choose a reason for hiding this comment

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

The comment made by the bot after workflow run, shouldn't be hard-coded, instead it should take the values from a json file or array that contains all the respective triager maintianers.

.github/workflows/notify-docs-triager.yml Outdated Show resolved Hide resolved
@anshgoyalevil
Copy link
Member

anshgoyalevil commented Feb 24, 2024

Everything seems to be working except the fact that we do not have write permissions allowed for GH Actions in this repo
Resource not accessible by integration

image

@derberg I am not sure how AsyncAPI bot comments on PRs using this workflow https://github.com/asyncapi/website/blob/master/.github/workflows/lint-pr-title.yml when we don't have write permissions for GH Actions.

Do we need to add this workflow to https://github.com/asyncapi/.github/ for it to have adequate permissions?

// cc @akshatnema

@TRohit20
Copy link
Collaborator Author

@akshatnema Fixed.

@sambhavgupta0705 The workflow is currently under .github/workflows as it should be and the file placement is not a issue here. The secret in context needs to have pull request read/write access. Your question with lint PR workflow is valid to an extent. If we don't want to give the secret write access as GITHUB_TOKEN can be used in other workflows, we could create a secret called "WORKFLOW_SECRET" with write access and use the secret just for this workflow. It's not optimal though.

What do you think @derberg

@akshatnema
Copy link
Member

akshatnema commented Feb 26, 2024

@asyncapi-bot
Copy link
Contributor

Hello, @akshatnema! 👋🏼

    I'm 🧞🧞🧞 Genie 🧞🧞🧞 from the magic lamp. Looks like somebody needs a hand!

    At the moment the following comments are supported in pull requests:

    - `/ready-to-merge` or `/rtm` - This comment will trigger automerge of PR in case all required checks are green, approvals in place and do-not-merge label is not added
    - `/do-not-merge` or `/dnm` - This comment will block automerging even if all conditions are met and ready-to-merge label is added
    - `/autoupdate` or `/au` - This comment will add `autoupdate` label to the PR and keeps your PR up-to-date to the target branch's future changes. Unless there is a merge conflict or it is a draft PR.

@sambhavgupta0705
Copy link
Member

Will this work on issues also because I need to triage issues also

@TRohit20
Copy link
Collaborator Author

@sambhavgupta0705 Sure, I mean we'd probably have to add another check and event trigger to see and make it work on issues too. Possible

@sambhavgupta0705
Copy link
Member

@TRohit20 is this ready for review?

@TRohit20
Copy link
Collaborator Author

TRohit20 commented Mar 5, 2024

@sambhavgupta0705 As mentioned above, the secret needs PR comment/write access to make a comment as intended, the other workflow suggested to try by @akshatnema also didn't quite seem to work.

Copy link
Member

Hmm @akshatnema would a short call help?

@Shurtu-gal
Copy link
Contributor

Everything seems to be working except the fact that we do not have write permissions allowed for GH Actions in this repo Resource not accessible by integration

image

@derberg I am not sure how AsyncAPI bot comments on PRs using this workflow https://github.com/asyncapi/website/blob/master/.github/workflows/lint-pr-title.yml when we don't have write permissions for GH Actions.

Do we need to add this workflow to https://github.com/asyncapi/.github/ for it to have adequate permissions?

// cc @akshatnema

Yeah, it needs to be added there, as far as I believe.
Better would be to create a dummy PR your own repository itself, and check if things are working.

triagers.json Outdated Show resolved Hide resolved
@TRohit20
Copy link
Collaborator Author

TRohit20 commented Mar 12, 2024

@Shurtu-gal The GitHub script being used in latest commit is actually a reuse of what we currently have in one of our workflows as suggested by @akshatnema

.github/workflows/notify-triager.yml Outdated Show resolved Hide resolved
.github/workflows/notify-triager.yml Outdated Show resolved Hide resolved
Copy link
Member

@anshgoyalevil anshgoyalevil left a comment

Choose a reason for hiding this comment

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

All parts are working great (ref). LGTM 🚀

@akshatnema
Copy link
Member

/rtm

@asyncapi-bot asyncapi-bot merged commit ff5fb74 into asyncapi:master Mar 16, 2024
22 checks passed
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.

Triage maintainers are not pinged in relevant PRs
8 participants