Skip to content

Split workflow in several hooks #165

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

Merged
merged 18 commits into from
Sep 13, 2019
Merged

Split workflow in several hooks #165

merged 18 commits into from
Sep 13, 2019

Conversation

La0
Copy link
Contributor

@La0 La0 commented Sep 10, 2019

Fixes #14

So we can work on #150

@La0 La0 added the bot Python Taskcluster Bot label Sep 10, 2019
@La0 La0 requested a review from marco-c September 10, 2019 13:50
@La0 La0 self-assigned this Sep 10, 2019
@marco-c
Copy link
Collaborator

marco-c commented Sep 10, 2019

Could you split it in actual separate Taskcluster hooks? One for cron, one triggered by pulselistener. Then, you could add an argument to cli.py to choose the workflow, to make it less brittle than relying on the values of other args (https://github.com/mozilla/code-coverage/pull/165/files#diff-6310014f6dc184cfe909a867e61bfcb8R64-R77).

Nit: I'd rename "mc.py" and "try_repo.py" to make it clear what they do (e.g. if we support other repos in the future, "mc.py" would not make sense anymore). "mc.py" is the task that runs at the end of a full test run, "try_repo.py" is the one that runs on-demand when developers push to try. I'm not sure what a good naming would be, maybe tests.py/all_tests.py/full_tests.py/push_tests.py/push.py/merge.py/merge_tests.py and on_demand.py?

@La0
Copy link
Contributor Author

La0 commented Sep 11, 2019

I went further than your comment, and used only 2 hooks:

  1. code_review_bot.hooks.repo for all repositories, using a static configuration to change behaviour according to the repository in use
  2. code_review_bot.hooks.cron for a scheduled task

I prefered to use 2 different endpoints, as some CLI options are not used by the cron worfklow.
The CLI & TC setup code is shared by both endpoints though.

In the end, i think the code is a bit cleaner, as we now only have 2 different worfklows (one being parametrizable), and we can support other repos a bit more easily.

Copy link
Collaborator

@marco-c marco-c left a comment

Choose a reason for hiding this comment

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

Great cleanup!

@La0 La0 merged commit 9c8c181 into mozilla:master Sep 13, 2019
@La0 La0 deleted the bot-split branch September 13, 2019 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot Python Taskcluster Bot
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Split codecoverage bot into multiple hooks
2 participants