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

feat: Ensuring that PRs are linked and estimated #9

Merged
merged 21 commits into from
Jan 25, 2023

Conversation

mtrunkat
Copy link
Member

@mtrunkat mtrunkat commented Jan 21, 2023

It simply checks the following 2 conditions:

  • PR is linked to the issue or is linked to epic or is labeled as adhoc
  • PR is estimated or its linked issue is estimated

@github-actions github-actions bot added this to the 55th sprint - Platform team milestone Jan 21, 2023
@github-actions github-actions bot added the t-platform Issues with this label are in the ownership of the platform team. label Jan 21, 2023
Copy link

@github-actions github-actions bot left a 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!

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Pull Request Tookit has failed!

Pull request is neither linked to an issue or epic nor labeled as adhoc!

@mtrunkat mtrunkat added the adhoc Ad-hoc unplanned task added during the sprint. label Jan 21, 2023
@mtrunkat mtrunkat removed the adhoc Ad-hoc unplanned task added during the sprint. label Jan 21, 2023
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Pull Request Tookit has failed!

Pull request is neither linked to an issue or epic nor labeled as adhoc!

@mtrunkat mtrunkat added the adhoc Ad-hoc unplanned task added during the sprint. label Jan 21, 2023
Copy link

@github-actions github-actions bot left a 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!

@mtrunkat
Copy link
Member Author

Test

@mtrunkat mtrunkat added adhoc Ad-hoc unplanned task added during the sprint. and removed adhoc Ad-hoc unplanned task added during the sprint. labels Jan 21, 2023
@mtrunkat mtrunkat requested review from fnesveda and drobnikj January 21, 2023 11:37
@mtrunkat
Copy link
Member Author

@fnesveda @drobnikj, how would you rate the annoyance of this check?

There is one benefit for everybody:

  • With this, there won't be any need for fixes for the computation of capitalization at the end of the quarter.

One befit for me and the team leads:

  • We will have an exact overview of an unplanned work

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.

Copy link
Member

@fnesveda fnesveda left a 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.

package-lock.json Outdated Show resolved Hide resolved
src/helpers.ts Outdated Show resolved Hide resolved
src/helpers.ts Outdated Show resolved Hide resolved
@mtrunkat
Copy link
Member Author

@fnesveda

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

Could you help me to brainstorm how to do this effectively? :)
I could simply sleep for 2 mins. But that will consume units, right?

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 1000 * 0.008 * 2 / 3 = $5 per month which is fine I guess.

I tried to implement this with stupid simple logic: a120b2b#diff-4fab5baaca5c14d2de62d8d2fceef376ddddcc8e9509d86cfa5643f51b89ce3dR52

I would not do this for draft PRs, only when they're marked as ready for review

Done

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

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.

@mtrunkat mtrunkat requested a review from fnesveda January 23, 2023 15:20
@mtrunkat mtrunkat removed the adhoc Ad-hoc unplanned task added during the sprint. label Jan 23, 2023
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Pull Request Tookit has failed!

Pull request is neither linked to an issue or epic nor labeled as adhoc!

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Pull Request Tookit has failed!

Pull request is neither linked to an issue or epic nor labeled as adhoc!

@mtrunkat mtrunkat added the debt Code quality improvement or decrease of technical debt. label Jan 23, 2023
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Pull Request Tookit has failed!

Pull request is neither linked to an issue or epic nor labeled as adhoc!

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Pull Request Tookit has failed!

Pull request is neither linked to an issue or epic nor labeled as adhoc!

@mtrunkat mtrunkat added adhoc Ad-hoc unplanned task added during the sprint. and removed debt Code quality improvement or decrease of technical debt. labels Jan 23, 2023
Copy link
Member

@drobnikj drobnikj left a 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.

Copy link
Member

@fnesveda fnesveda left a 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.

@mtrunkat mtrunkat merged commit 3114e24 into main Jan 25, 2023
@mtrunkat mtrunkat deleted the feature/ensure-linking-and-estimates branch January 25, 2023 08:34
@fnesveda fnesveda added the validated Issues that are resolved and their solutions fulfill the acceptance criteria. label Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adhoc Ad-hoc unplanned task added during the sprint. t-platform Issues with this label are in the ownership of the platform team. validated Issues that are resolved and their solutions fulfill the acceptance criteria.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants