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

Add refresh back-off period configuration #2171

Merged

Conversation

javierarrieta
Copy link
Contributor

Add refresh back-off period configuration

Fixes #1912

@@ -58,7 +58,8 @@ object Cli {
githubAppKeyFile: Option[File] = None,
githubAppId: Option[Long] = None,
urlCheckerTestUrl: Option[Uri] = None,
defaultMavenRepo: Option[String] = None
defaultMavenRepo: Option[String] = None,
refreshBackoffPeriod: Option[FiniteDuration] = None
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure if set the default at this level or when creating the Config. Might make more sense to set it here than where I wrote it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most default values are set here, so I guess it makes sense to also set it here for refreshBackoffPeriod.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I will change that, thanks!

@javierarrieta
Copy link
Contributor Author

I will add the config to the docs when we agree on the approach

Copy link
Member

@fthomas fthomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM, many thanks @javierarrieta.

So to disable any refresh backoff, one just passes --refresh-backoff-period 0days as CLI option.

@@ -58,7 +58,8 @@ object Cli {
githubAppKeyFile: Option[File] = None,
githubAppId: Option[Long] = None,
urlCheckerTestUrl: Option[Uri] = None,
defaultMavenRepo: Option[String] = None
defaultMavenRepo: Option[String] = None,
refreshBackoffPeriod: Option[FiniteDuration] = None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most default values are set here, so I guess it makes sense to also set it here for refreshBackoffPeriod.

@fthomas fthomas added the enhancement New feature or request label Jun 30, 2021
@fthomas fthomas added this to the 0.11.0 milestone Jun 30, 2021
@javierarrieta
Copy link
Contributor Author

This LGTM, many thanks @javierarrieta.

So to disable any refresh backoff, one just passes --refresh-backoff-period 0days as CLI option.

Yes, I will add it to the docs in this PR, let's see if I fixed the build

@codecov
Copy link

codecov bot commented Jun 30, 2021

Codecov Report

Merging #2171 (4d5365b) into master (fcb3205) will increase coverage by 0.00%.
The diff coverage is 60.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2171   +/-   ##
=======================================
  Coverage   78.16%   78.17%           
=======================================
  Files         131      131           
  Lines        2249     2250    +1     
  Branches       57       46   -11     
=======================================
+ Hits         1758     1759    +1     
  Misses        491      491           
Impacted Files Coverage Δ
.../scala/org/scalasteward/core/application/Cli.scala 96.15% <ø> (ø)
.../scalasteward/core/repocache/RefreshErrorAlg.scala 6.66% <0.00%> (ø)
...ala/org/scalasteward/core/application/Config.scala 97.36% <100.00%> (+0.07%) ⬆️
...la/org/scalasteward/core/application/Context.scala 84.74% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fcb3205...4d5365b. Read the comment docs.

@javierarrieta
Copy link
Contributor Author

Documentation added to the help.md file, would that be enough?

@fthomas
Copy link
Member

fthomas commented Jun 30, 2021

Documentation added to the help.md file, would that be enough?

Yes, thanks!

@fthomas fthomas merged commit 92f43ce into scala-steward-org:master Jun 30, 2021
@javierarrieta javierarrieta deleted the refresh_backoff_configuration branch June 30, 2021 13:36
@javierarrieta
Copy link
Contributor Author

Thanks for the quick turnaround and mostly thanks for this great piece of software that help many of us with their day to day

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[on-prem] Update is skipped completely if there was previous issue
2 participants