chore!: enable var-naming from revive#2960
chore!: enable var-naming from revive#2960mmorel-35 wants to merge 1 commit intotestcontainers:mainfrom
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
stevenh
left a comment
stevenh
left a comment
There was a problem hiding this comment.
Quite a few breaking changes here @mdelapenya due to public method renaming, thoughts?
| arguments: | ||
| - ["ID"] # AllowList | ||
| - ["VM"] # DenyList | ||
| - - upperCaseConst: true # Extra parameter (upperCaseConst|skipPackageNameChecks) |
There was a problem hiding this comment.
question: is the second dash required, looks like a typo?
There was a problem hiding this comment.
No it’s not a typo, see this https://golangci-lint.run/usage/linters/#revive
| func (c *ConsulContainer) ApiEndpoint(ctx context.Context) (string, error) { | ||
| mappedPort, err := c.MappedPort(ctx, defaultHttpApiPort) | ||
| // APIEndpoint returns host:port for the HTTP API endpoint. | ||
| func (c *ConsulContainer) APIEndpoint(ctx context.Context) (string, error) { |
There was a problem hiding this comment.
suggestion: this is breaking change so needs ! added to the title and "BREAKING CHANGE" to the description, see conventional commits for details
mmorel-35
changed the title
mdelapenya
changed the title
mmorel-35
force-pushed
the
revive/var-naming
branch
2 times, most recently
from
ff493de to
3f5ac46
Compare
mdelapenya
left a comment
mdelapenya
left a comment
There was a problem hiding this comment.
I'm tempted to block this PR until we release a v1. Almost all the modules will be affected, impacting lots of potential users of the library.
We already have some BC in the current release, some more in the previous one, but they were very narrowed. Here we are not offering a deprecation path for the renamed methods.
Wdyt?
|
I can understand, |
|
Thanks for understanding, and your work!, @mmorel-35, it's a pleasure working with engineers like you. If we can do that, first tackling the private fields, then we can move on with an another round of changes for the public API. We can keep this PR as reference, in draft, and send different chunks. Thoughts? |
|
Of course, I'll provide another branch to fit what we agree on. |
Signed-off-by: Matthieu MOREL <[email protected]>
mmorel-35
force-pushed
the
revive/var-naming
branch
from
3f5ac46 to
747e597
Compare
3 participants
What does this PR do?
enable var-naming rule from revive
This also moves output configuration from Makefile to golangci-lint config
BREAKING CHANGES