Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deadline is now deprecated, but should be taking its value from the configuration if set #822

Merged
merged 5 commits into from Oct 15, 2019

Conversation

dcaba
Copy link
Contributor

@dcaba dcaba commented Oct 15, 2019

Hi,

I'm afraid the PR #793 generated a side effect: while deadline still works and becomes the effective timeout setting when supplying it as a flag/cmdline arg, we've realized deadline in the configuration file is no longer being honored (ok, its deprecated, but probably we wanted this to become a breaking change, forcing all consumers to immediately change their config): see the test we are incorporating, that clearly reproduces the situation.

Proposing a fix (but not prod of it), so deadline is still considered (no matter where it comes from) if its value is different of the default timeout (that becomes a constant). There may be alternative implementations or better places to put this logic, but I didnt want to make the config package aware of the default value...

Sorry if the explanation is not clear: let me know if that is the case and I will do my best to clarify.

@CLAassistant
Copy link

CLAassistant commented Oct 15, 2019

CLA assistant check
All committers have signed the CLA.

…es from config, is 0. Plus static check is going to fail because of deprecated cfg used
@dcaba dcaba changed the title deadline is now deprecated, but taking its value from the configuration if set deadline is now deprecated, but should be taking its value from the configuration if set Oct 15, 2019
@jirfag jirfag merged commit 98f60eb into golangci:master Oct 15, 2019
@jirfag
Copy link
Member

jirfag commented Oct 15, 2019

thank you!

jirfag pushed a commit that referenced this pull request Oct 15, 2019
…onfiguration if set (#822)

* test that demostrates that deadline is not working if comes from the config

* overriding timeout with deadline when only deadline is different from its default value

* tests were not passing. default value for Deadline, that now only comes from config, is 0. Plus static check is going to fail because of deprecated cfg used

* golangci should use the latest golangci-lint version, that is the one deprecating deadline in favour of timeout

* README updated - looks the ci config in this project is used to generate usage instructions.. great!
melinath added a commit to hashicorp/terraform-provider-google that referenced this pull request Jul 29, 2022
melinath added a commit to hashicorp/terraform-provider-google-beta that referenced this pull request Jul 29, 2022
melinath added a commit to hashicorp/terraform-provider-google-beta that referenced this pull request Jul 29, 2022
melinath added a commit to hashicorp/terraform-provider-google that referenced this pull request Jul 29, 2022
@ldez ldez added this to the v1.21 milestone Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants