Skip to content

tests: refactor py_reconfig rules so less boilerplate is needed to add attrs #2933

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 2 commits into from
May 24, 2025

Conversation

rickeylev
Copy link
Collaborator

Just some minor refactoring of the py_reconfig rule so that it's easier to add attributes
that affect transition state. After this, just two spots have to be modified to
add an attribute (map of attrs, map of attr to transition label).

@rickeylev rickeylev requested review from groodt and dougthor42 May 24, 2025 19:10
@rickeylev rickeylev requested a review from aignas as a code owner May 24, 2025 19:10
@rickeylev rickeylev enabled auto-merge May 24, 2025 19:10
@dougthor42 dougthor42 changed the title tests: refacto py_reconfig rules so less boilerplate is needed to add attrs tests: refactor py_reconfig rules so less boilerplate is needed to add attrs May 24, 2025
Copy link
Collaborator

@dougthor42 dougthor42 left a comment

Choose a reason for hiding this comment

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

I can't say that I fully understand it, but I don't see any glaring issues. So LGTM.

This facilitates verify running binaries with different outer environmental
settings and verifying their output without the overhead of a bazel-in-bazel
integration test.
This facilitates verify running binaries with different configuration settings
Copy link
Collaborator

Choose a reason for hiding this comment

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

This verbiage is a little odd to me. Is verify a command / action? If so, maybe add quotes or a reference?

This facilitates "verify" (the action) running binaries with different configuration settings

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, that's a typo. It should say something more like

This facilitates running binaries ... and verifying their output ...

@rickeylev rickeylev added this pull request to the merge queue May 24, 2025
Merged via the queue into bazel-contrib:main with commit 28fda86 May 24, 2025
3 checks passed
@rickeylev rickeylev deleted the tests.dry.reconfig branch May 27, 2025 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants