Refactors output_field to default to something that disambiguates on paramtrized fields
#16232
Conversation
|
A cursory glance didn't show anywhere to test the packaging behaviour for regression (golang's |
…n parametrized fields [ci skip-rust] [ci skip-build-wheels]
chrisjrn
force-pushed
the
chrisjrn/16189-whoops-colliding-packages
branch
from
1642771
to
0cd8573
Compare
| f"{sanitize(k)}={sanitize(v)}" for k, v in self.parameters.items() | ||
| ) | ||
| params = f"@@{key_value_strs}" | ||
| params = f"@{sanitize(self.parameters_repr)}" |
There was a problem hiding this comment.
This is unrelated to the refactor, but it does make path_safe_spec slightly easier to read
There was a problem hiding this comment.
Does it change the actual value? Or solely code factoring? If it changes behavior, it would be better as a dedicated change.
| f"{sanitize(k)}={sanitize(v)}" for k, v in self.parameters.items() | ||
| ) | ||
| params = f"@@{key_value_strs}" | ||
| params = f"@{sanitize(self.parameters_repr)}" |
There was a problem hiding this comment.
Does it change the actual value? Or solely code factoring? If it changes behavior, it would be better as a dedicated change.
It doesn't change the actual value, just how it's calculated. |
| @@ -72,11 +72,14 @@ class OutputPathField(StringField, AsyncFieldMixin): | |||
| def value_or_default(self, *, file_ending: str | None) -> str: | |||
There was a problem hiding this comment.
Re testing: this would be good to unit test in a new file package_test.py. We shouldn't have done that for a long time, bad on us
|
Created #16247 as future work to address lack of unit tests |
…mbiguates on parametrized fields (pantsbuild#16232) (cherry picked from commit 28d7af5) # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust]
…mbiguates on parametrized fields (pantsbuild#16232) # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
…mbiguates on parametrized fields (pantsbuild#16232) # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
Resolves #16189