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

Error out on young PRs without fast-track label #131

Closed
BridgeAR opened this issue Dec 12, 2017 · 6 comments
Closed

Error out on young PRs without fast-track label #131

BridgeAR opened this issue Dec 12, 2017 · 6 comments
Labels
feature request New features for node-core-utils pr-checker Issues related to pr checker

Comments

@BridgeAR
Copy link
Member

If a PR has a fast-track label on it, it should not be complained about. Right now there is a warning sign in case the PR is younger than 48 hours (do we check 72 hours on the weekend by the way?). But if the label is applied, that should not warn out of my perspective. And if the label is not applied, I would expect a error instead of just a warning.

@priyank-p
Copy link
Contributor

@BridgeAR we do check for 72 hours on weekdays.

And if the label is not applied, I would expect a error instead of just a warning.

I dont think we can detect if a PR needs fast track

Can you elaborate more on what you need error for or don not have warning for.

@apapirovski
Copy link
Member

I dont think we can detect if a PR needs fast track

We definitely can, that's what the fast-track label is for.

@priyank-p
Copy link
Contributor

@apapirovski we do warn in case when PR has fast-track label i think @BridgeAR was talking about detecting if PR needs fast-tracking

(Correct me if am wrong)

@apapirovski
Copy link
Member

In that case there doesn't seem to be much to do with this issue? I think @BridgeAR is just asking for the functionality that was implemented in https://github.com/nodejs/node-core-utils/pull/104/files

@BridgeAR
Copy link
Member Author

What I am asking for is a error instead of a warning. It is of course trivial to change that but I would really like to enforce using the label.

@joyeecheung joyeecheung added the feature request New features for node-core-utils label Dec 18, 2017
@joyeecheung joyeecheung added the pr-checker Issues related to pr checker label Jan 18, 2018
@joyeecheung joyeecheung changed the title Detect fast tracking Error out on non-fast-track PRs that's too young Jan 18, 2018
@joyeecheung joyeecheung changed the title Error out on non-fast-track PRs that's too young Error out on young PRs without fast-track label Jan 18, 2018
srl295 added a commit to srl295/node-core-utils that referenced this issue Oct 5, 2018
Clarify that the wait time is a *minimum* not a maximum. Also see  nodejs#131
@mmarchini
Copy link
Contributor

ncu handles fast-track properly today

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New features for node-core-utils pr-checker Issues related to pr checker
Projects
None yet
Development

No branches or pull requests

5 participants