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 the label-on-approval workflow #6136

Merged
merged 1 commit into from
May 19, 2024
Merged

Conversation

necolas
Copy link
Contributor

@necolas necolas commented May 18, 2024

Resolves issues with #6120 (probably)

Copy link

vercel bot commented May 18, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
lexical ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 18, 2024 4:40pm
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 18, 2024 4:40pm

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 18, 2024
Copy link

github-actions bot commented May 18, 2024

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
packages/lexical/dist/Lexical.js 23.91 KB (0%) 479 ms (0%) 116 ms (+11.58% 🔺) 594 ms
packages/lexical-rich-text/dist/LexicalRichText.js 34.6 KB (0%) 693 ms (0%) 217 ms (-15.21% 🔽) 909 ms
packages/lexical-plain-text/dist/LexicalPlainText.js 34.51 KB (0%) 691 ms (0%) 298 ms (+8.24% 🔺) 988 ms

@Sahejkm
Copy link
Contributor

Sahejkm commented May 18, 2024

  • I don't think this works and auto triggers e2e tests, 1 GitHub workflow can't trigger another GitHub workflow as per the docs unless we use a pat token to run add label job.

  • 6120 is working fine for meta employees but not for external reviews becoz the GitHub actions don't have permissions for fork PRs and need pull request target event to work

@necolas
Copy link
Contributor Author

necolas commented May 18, 2024

Got it. Updated to rename jobs for clarity (since it's less about labels and more about what happens after approval), and adopted the alternative trigger that you said would work for PRs from forks

Sahejkm
Sahejkm previously approved these changes May 18, 2024
@Sahejkm
Copy link
Contributor

Sahejkm commented May 18, 2024

  • I think it broke :( , I will test and create a pr for this as it was ald on my mind 😀

  • we have to give both pull request and pull request target

  • we should specify branch also when using pull request target

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@necolas
Copy link
Contributor Author

necolas commented May 18, 2024

Updated

Copy link
Contributor Author

@necolas necolas left a comment

Choose a reason for hiding this comment

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

test review

Copy link
Contributor

@Sahejkm Sahejkm left a comment

Choose a reason for hiding this comment

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

Thanks @necolas for enhancing this!

@github-actions github-actions bot added the extended-tests Run extended e2e tests on a PR label May 19, 2024
@Sahejkm Sahejkm added this pull request to the merge queue May 19, 2024
Merged via the queue into main with commit 58853cc May 19, 2024
51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. extended-tests Run extended e2e tests on a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants