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.

Sign up for GitHub

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

Jump to bottom

Some minor cleanup to 'guess_worker_impl' #598

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Some minor cleanup to 'guess_worker_impl' #598

wants to merge 2 commits into from

Conversation

ahal
Copy link
Contributor

@ahal ahal commented Mar 13, 2023

No description provided.

@ahal
refactor(cot_verify): remove unnecessary check in guess_worker_impl
c9d2625 
The scriptworker-prov-v1 provisioner is already defined in constants.py,
so no need to check for it explicitly.
@ahal ahal requested a review from a team March 13, 2023 20:22
@ahal ahal self-assigned this Mar 13, 2023
hneiva
hneiva approved these changes Mar 13, 2023
View reviewed changes
Copy link
Contributor

@hneiva hneiva left a comment

Choose a reason for hiding this comment

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

LGTM

bhearsum
bhearsum approved these changes Mar 13, 2023
View reviewed changes
Copy link
Contributor

@bhearsum bhearsum left a comment

Choose a reason for hiding this comment

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

Just to be clear: this wouldn't fix the issue we just had with langpacks (that resulted in https://phabricator.services.mozilla.com/D172475 as a workaround) - right? (This is purely just cleanup, that should have no impacts.)

@ahal
Copy link
Contributor Author

ahal commented Mar 13, 2023

This is purely just cleanup, that should have no impacts

Correct, well other than all the tests I broke ><

@ahal ahal marked this pull request as draft March 13, 2023 20:36
@ahal
Copy link
Contributor Author

ahal commented Mar 13, 2023

Sigh, I guess that explicit scriptworker-prov-v1 check was left in to keep the tests happy. Ideally we'd update the tests to use a dummy configuration, but I'm not sure it's worth it.. may just end up abandoning this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hneiva hneiva hneiva approved these changes

@bhearsum bhearsum bhearsum approved these changes

Assignees

@ahal ahal

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ahal @bhearsum @hneiva