-
Notifications
You must be signed in to change notification settings - Fork 29
Refactor rules into configurable phases #141
Conversation
… and whatever CI uses
(I think there's one lingering issue where the SingleJar is combining files in a different order than we originally expected, but only in CI.) |
# phase will merge it into the final jar. | ||
# | ||
|
||
def phase_resources(ctx, g): |
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.
Having a big comment block before each of these phases makes me think they should probably each be in their own file.
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 think so too. I have a few more changes in other PRs to iterate on. These will influence the phase names and interfaces (in the global g
struct), so I'll want to do wrap them up before splitting phases to separate files. I'd also like to organize the associated Scala source files along with the phases: the deps checker Scala can sit in the same area as the deps checker Starlark code.
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 to me.
Looks good to me. One question though, are we planning on doing similar thing to proto and format? |
I don't think the proto and format rules share the same phases. If they do, then we could perhaps do something similar down the road. |
I am going to go ahead and merge this one, as it heavily impacts future work. |
This refactors the core rules into a series of more isolated and configurable phases that can be run together as a pipeline. Additionally, it enables plugins to insert new phases into the pipeline for a particular rule.
This should enable the following future work: