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

Update cron-utils to 9.2.1 #864

Merged
merged 2 commits into from
Aug 24, 2023
Merged

Update cron-utils to 9.2.1 #864

merged 2 commits into from
Aug 24, 2023

Conversation

minwoox
Copy link
Member

@minwoox minwoox commented Aug 21, 2023

I'm not sure why we haven't upgrade cron-utils to the latest. There's no reason not to upgrade it.

cron-utils: 5.0.5 -> 9.2.1

I'm not sure why we haven't upgrade cron-utils to the latest.
There's no reason not to upgrade it.

cron-utils: 5.0.5 -> 9.2.1
@codecov
Copy link

codecov bot commented Aug 21, 2023

Codecov Report

Patch coverage: 50.00% and project coverage change: +0.02% 🎉

Comparison is base (07a1fa6) 65.50% compared to head (47a6a5e) 65.53%.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #864      +/-   ##
============================================
+ Coverage     65.50%   65.53%   +0.02%     
- Complexity     3328     3331       +3     
============================================
  Files           356      356              
  Lines         13962    13970       +8     
  Branches       1499     1500       +1     
============================================
+ Hits           9146     9155       +9     
+ Misses         3964     3960       -4     
- Partials        852      855       +3     
Files Changed Coverage Δ
...erver/internal/mirror/DefaultMirroringService.java 65.62% <40.00%> (-1.77%) ⬇️
...aldogma/server/internal/mirror/AbstractMirror.java 91.37% <57.14%> (-4.92%) ⬇️

... and 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@minwoox minwoox added this to the 0.62.0 milestone Aug 23, 2023
Copy link
Contributor

@ikhoon ikhoon left a comment

Choose a reason for hiding this comment

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

Thanks!

return next.get().plus(jitterMillis, ChronoUnit.MILLIS);
}
throw new IllegalArgumentException(
"no next execution time for " + CRON_DESCRIPTOR.describe(schedule) + ", lastExecutionTime: " +
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Should we log localRepo as well to inform the errors to users.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was also thinking about logging the information but it's logged here so I didn't include it. 😉

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense.

Copy link
Contributor

@ikhoon ikhoon left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Contributor

@jrhee17 jrhee17 left a comment

Choose a reason for hiding this comment

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

Thanks! 👍 👍 👍
I think this would be a good release to add monitoring dashboards for mirroring as well. I'll add this before this release

Copy link
Member

@trustin trustin left a comment

Choose a reason for hiding this comment

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

Thanks for keeping it up-to-date!

@trustin trustin merged commit 12fea92 into line:main Aug 24, 2023
8 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants