-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: Ensuring that PRs are linked and estimated #9
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request is neither linked to an issue or epic nor labeled as adhoc!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request is neither linked to an issue or epic nor labeled as adhoc!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request is neither linked to an issue or epic nor labeled as adhoc!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If issue is not linked to the pull request then estimate the pull request!
Test |
@fnesveda @drobnikj, how would you rate the annoyance of this check? There is one benefit for everybody:
One befit for me and the team leads:
But it might be slightly annoying. You create a PR and get an error quickly when you setup something badly. Perhaps we could add a 30 seconds sleep to the first run for each PR so that the developer gets time to configure everything (link issue/epic and estimate) after the PR gets created. Perhaps we should not run this on DRAFTs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this, it's nice that we won't have to do capitalization 3 months later, when I sometimes don't even remember what the PR was about.
Some general points:
- I would definitely add a delay to the first run, maybe even 2 minutes, to have time to find the epic/issue and link it
- I would not do this for draft PRs, only when they're marked as ready for review
- I'm not sure about writing the review as comment, maybe having the action fail would be enough
- every other reviewer will get a notification about the comment too, not just the PR author
- the PR author will get 2 notifications - one about the failing action and one about the comment
- then the review will just be left hanging there, maybe it could be deleted once the action passes?
- it will be slightly annoying that you'll have to rerun the action manually when you link an issue / add an estimate, because the workflow won't be rerun on its own
And I left some small comments in the files too.
Co-authored-by: František Nesveda <[email protected]>
Could you help me to brainstorm how to do this effectively? :) But in fact, with the base 2 core actions on Linux, it's $0.008/minute. And we have <1000 PRs per quarter. So if we execute these only once with sleep for each PR then it's I tried to implement this with stupid simple logic: a120b2b#diff-4fab5baaca5c14d2de62d8d2fceef376ddddcc8e9509d86cfa5643f51b89ce3dR52
Done
Y. Two notifications are annoying but on the other hand, it's easier for me to see the comment rather than open an action log. Not sure here. Ideally, you should be always able to pass this and beware of them so I'd now focus on point one giving you time to sent this up correctly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request is neither linked to an issue or epic nor labeled as adhoc!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request is neither linked to an issue or epic nor labeled as adhoc!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request is neither linked to an issue or epic nor labeled as adhoc!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request is neither linked to an issue or epic nor labeled as adhoc!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will probably be annoying, but better than fixing it later on a quarterly basis.
One idea, can we skip PR, where are no reviewers? But not sure if it is possible.
The reason is that we probably set up labels at the same time we are adding reviewers.
Otherwise, the code looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Let's try it like this, I think it should be fine, and if it's not, we can improve it.
It simply checks the following 2 conditions:
adhoc