cli: deprecate cli.StatusError direct field usage#5738
cli: deprecate cli.StatusError direct field usage#5738Benehiko wants to merge 1 commit intodocker:masterfrom
cli.StatusError direct field usage#5738Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5738 +/- ##
==========================================
+ Coverage 59.51% 59.53% +0.01%
==========================================
Files 346 346
Lines 29376 29378 +2
==========================================
+ Hits 17483 17490 +7
+ Misses 10923 10918 -5
Partials 970 970 |
The exported `cli.StatusError` type may be used
by some directly within their own projects, making
it difficult to update the struct's fields.
This patch converts the exported `cli.StatusError` to
an interface instead, so that code wrapping the CLI
would still be able to match the error and get the
status code without exposing the fields.
This is a breaking change for those relying on creating
a `cli.StatusError{}` and accessing the error's fields.
For those using `errors.As(err, &statusError)` and
`err.(cli.StatusError)` will be able to continue using
it without breakage.
Users accessing the fields of `cli.StatusError{}.StatusCode`
would need to use the new `GetStatusCode()` method.
Signed-off-by: Alano Terblanche <[email protected]>
Benehiko
force-pushed
the
deprecate-status-error
branch
from
e6f65c4 to
5063fa3
Compare
IMO turning this into an interface is fine (and I'm happy it matches what we do elsewhere – see However, turning this into an interface is still a breaking change, so it can't go out in a minor release. If we're breaking it anyway, it might be worth to go with a name we like/that matches the others, such as This is still something we can do in the meantime as I mentioned in the other PR. |
|
Yeah, this PR is not meant to go into a minor release and can be merged whenever. I made this PR to do a singular change to This PR is essentially more so to force users to not use the |
I'm not sure whether this means it can be merged whenever or not but my point is that the same conditions under which this PR can be merged would allow for more substantial changes, so I'm not sure I understand the reasoning for a half-measure. |
|
Closing this PR and moving the commit back to the original PR since most of the discussion happened there #5666 |
3 participants
The exported
cli.StatusErrortype may be usedby some directly within their own projects, making it difficult to update the struct's fields.
This patch converts the exported
cli.StatusErrorto an interface instead, so that code wrapping the CLI would still be able to match the error and get the status code without exposing the fields.This is a breaking change for those relying on creating a
cli.StatusError{}and accessing the error's fields. For those usingerrors.As(err, &statusError)anderr.(cli.StatusError)will be able to continue using it without breakage.Users accessing the fields of
cli.StatusError{}.StatusCodewould need to use the newGetStatusCode()method.- What I did
- How I did it
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)