search

uber-go / mock

GoMock is a mocking framework for the Go programming language.
Apache License 2.0
2.13k stars 109 forks source link

Deprecate the function ctrl.Finish() #50

Closed alexandear closed 1 year ago

alexandear commented 1 year ago

This PR marks the function ctrl.Finish as Deprecated. It's no need to call this function starting from Go 1.14 when T.Cleanup was added.

fifsky commented 1 year ago

If Finish is marked as Deprecated, a large amount of code that has already been written will not pass the golangci-lint check, which will take a long time to transform

bcho commented 1 year ago

If Finish is marked as Deprecated, a large amount of code that has already been written will not pass the golangci-lint check, which will take a long time to transform

I want to echo this. Instead of removing the Finish call directly, can we do a graceful deprecation? For example, we make this Finish call as no-op in the next version (may also print a log when calling this), then remove it in the next major release? This could provide a smoother migrate experience to the user.

Nivl commented 1 year ago

I want to echo this. Instead of removing the Finish call directly, can we do a graceful deprecation? For example, we make this Finish call as no-op in the next version (may also print a log when calling this), then remove it in the next major release? This could provide a smoother migrate experience to the user.

It's not being removed, it's being marked as deprecated.

If Finish is marked as Deprecated, a large amount of code that has already been written will not pass the golangci-lint check, which will take a long time to transform

This is fine and expected. People need to configure their linter correctly and plan their work accordingly.

Here's the issue. Deprecations are just notices that exist because removing a method would introduce a breaking change and people need time to update. A deprecation notice gives them the time they need. Now if we start using tools that break everything if we have a deprecation, that would make the whole deprecation thing useless.

marten-seemann commented 1 year ago

Please don't remove this function. It's needed to use gomock with certain testing frameworks, and removing the function would break those use cases.

Please also consider reverting the deprecation. If the function is not going to be removed in a future release, it shouldn't be marked deprecated in the first place.

See https://github.com/uber-go/mock/issues/81 for more details.