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

Don’t run bypass-checks on non "run" actions #3644

Merged
merged 8 commits into from Oct 14, 2020
Merged

Conversation

yakov116
Copy link
Member

@yakov116 yakov116 added the bug label Oct 13, 2020
@yakov116 yakov116 marked this pull request as draft October 13, 2020 03:05
@yakov116
Copy link
Member Author

yakov116 commented Oct 13, 2020

I dont like what I wrote. maybe will give it a shot in the morning. @fregante I am open to idea's.

@fregante
Copy link
Member

2. TEST URLS:
eslint/eslint#13748

Which one? I don't see any check that needs to be bypassed there.

@yakov116
Copy link
Member Author

Let me explain the issue.
eslint/eslint#13748 has a check for https://github.com/eslint/eslint-github-bot/blob/master/docs/commit-message-check.md and eslint/eslint#13719 (hover over the first commit status to see the second one)

These 2 are not action runs and they already link to their direct page.

@fregante
Copy link
Member

Use GitHub URL Detection with an argument, no need for custom reflexes unless you’re extracting data. In this case isActionRun(detailsLink) should work, except that the types don’t accept a raw HTMLAnchorElement. PR welcome there to change that

@fregante
Copy link
Member

fregante commented Oct 13, 2020

Also include the 2 types of valid run URLs in comments

@yakov116 yakov116 marked this pull request as ready for review October 14, 2020 02:37
source/features/bypass-checks.tsx Outdated Show resolved Hide resolved
source/features/bypass-checks.tsx Show resolved Hide resolved
@fregante
Copy link
Member

You can drop the package files, I updated the dependency in #3639

@yakov116 yakov116 merged commit c919488 into master Oct 14, 2020
@yakov116 yakov116 deleted the cant_bypass_em_all branch October 14, 2020 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

2 participants