Conversation
…e another build/test workflows will load revisions the same way that linting workflows do, but they will need to call a separate `Workflow` method. We'll be able to distinguish between the two by looking at the decision task parameters of the `try_group_id`. By putting these next to each other, we can call the right method without needing to leak the parameters out of this block into areas where they aren't appropriate.
While these are available in the `code-review` task metadata for the existing linting tasks, upcoming work to add builds and tests on phabricator revisions will not have such tasks. Luckily, all reviewbot pushes to Try already have `phabricator_diff` in their parameters, and we can fetch it from there. Note that while this makes `try_task_id` unneeded in `cli.py`, it's still needed in `workflow.py` to find the tasks that need to be analyzed.
The existing publication mode for linting checks requires this, but this won't exist when we start dealing with builds & tests (we'll be looking at the entire `try_group_id` instead). Given this, we can't require it to be there to have `try_group_id` set in the settings.
This requires two main things: 1) The ability to signal a mode when the bot starts 2) Raising the currently hardcoded taskgraph parameters up to `start_analysis`, to allow them to be set differently for different modes This will allow us to start additional analyses for builds, tests, etc. by setting the appropriate parameters when pushing to Try.
…sis modes This is similar to the previous commit in that we need a signal to the bot to indicate which mode to use, and to handle it. The details differ, however: * Our best signal for which mode to operate in comes from the decision task parameters, so that choice is deferred to cli.py. This is because the only way to process non-lint groups is to fire on task group completion (there will be no `code-review` task for these, and thus no way to feed that signal into the code review task payload itself). * `Workflow.run` is very clearly oriented specifically towards linting, and we'll need a substantially different version of it to process build and test results. For this reason, it's been moved entirely to its own method, with the intention of providing a new method when support for build and test results is added. This is different than `start_analysis`, where the only difference is in the parameters that get fed into the try push.
bhearsum
marked this pull request as ready for review
September 18, 2026 18:37
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a functionally equivalent refactor that is a useful precursor to https://bugzilla.mozilla.org/show_bug.cgi?id=2065267. It's best looked at commit-by-commit (each one has some additional details in the commit message). The high level overview is that it allows different code paths to be taken in analysis and publication mode that will allow us to push to try with different parameters, and analyze and publish results in different ways.
Perhaps the most notable part here is the adjustment of where
build_target_phidis fetched from. This is not strictly necessary as part of this refactor, but when we add a build&test try push, we won't have acode-reviewtask that we can fetch this from, and it's already available in the decision task parameters so I figured we may as well be consistent with where we fetch it from.This has been tested locally against phabricator-dev. I managed to kick off a sane looking try push, and when I re-ran publication mode against a previous code review bot run, it produced identical results.