feat(operations.brew, facts.brew): add support for brew trust#1836
feat(operations.brew, facts.brew): add support for brew trust#1836morrison12 wants to merge 14 commits into
Conversation
wowi42
added
new feature
facts
|
Ugh.. Forgot about python 3.10. Back in a bit. |
- change from StrEnum to plain Enum to make work with python 3.10 (and later) - rename to BrewItemKind for clarity
- brew.trust tests not needed in brew.tap
morrison12
changed the title
There was a problem hiding this comment.
Issues
-
trust() docstring example uses wrong parameter names. The example passes
item=(singular) but the parameter isitems=(plural), andtrust=Truebut the parameter istrusted. Copy-pasting the example would fail with TypeError. The+ item:docstring line also needs to become+ items:. -
raise OperationValueError from None has no message. When kind is an invalid string, the error is raised with no description. The user sees a bare OperationValueError with no clue what went wrong.
-
brew trust --json v1 uses space instead of =. The documented flag is --json=v1 (Homebrew 6.0.0 release notes). The canonical = form is safer and matches the upstream docs.
Notes
- trusted parameter docstring says "Default False" but the Python default is None. The
trusted = trusted or Falseline makes it behave as documented, but the docstring and signature are inconsistent.
|
|
||
| brew.trust( | ||
| name="Mark magic tap as trusted", | ||
| item="includeos/includeos", |
There was a problem hiding this comment.
The docstring example uses item= (singular) but the parameter is items= (plural), and trust=True but the parameter is trusted. Copy-pasting this example would fail with TypeError. The + item: docstring line above the example also needs to become + items:.
| try: | ||
| kind = BrewItemKind(kind) | ||
| except (TypeError, ValueError): | ||
| raise OperationValueError from None |
There was a problem hiding this comment.
raise OperationValueError from None has no message argument. When kind is an invalid string, the user sees a bare OperationValueError with no clue what went wrong. Add a descriptive message.
There was a problem hiding this comment.
I think the best course of action for this one is to wait until #1841 is merged and then I can update the tests and remove this per the TODO above it.
There was a problem hiding this comment.
code now removed.
|
|
||
| @override | ||
| def command(self) -> str: | ||
| return "brew trust --json v1" |
There was a problem hiding this comment.
The documented flag is --json=v1 (Homebrew 6.0.0 release notes, man page). While Homebrew may accept the space form, the canonical = form is safer and matches upstream docs.
- i.e. --json=v1 not json v1
- for brew.trusted, use items vs. item and use trusted vs. trust - for brew.tap set default for trust to False in param def; still need or False in body for when it is omitted
Also fixed.. The line in the body of the function is still required for the case where the parameter is not provided (needed for backwards compatibility). |
- use json=v1
- remove extraneous commas added by ruff
- remove extraneous commas added by ruff
- remove kludge allowing string values - update tests to use enum:BrewKind:<enum-value>
2 participants
Added support for the brew trust that is now mandatory with Homebrew 6.0 (and beyond) as noted in #1828.
Support is:
brew.tapas it seems unlikely one would want add a tap without trusting itbrew.trustoperation given one can choose to trust narrower scopes than the full tap, e.g. a specific cask.brew.BrewTrusted.The diffs are rather noisy given the changes required to appease the static checkers and my choice to change
brew.tapto useStringCommandet al.3.xat this time)scripts/dev-test.sh)scripts/dev-lint.sh)conventional commits format
ps ignore the mess from committing in the wrong place; I (think) I fixed that.