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

Retention: Align host filter with "backup" #227

Merged
merged 4 commits into from
Aug 7, 2023

Conversation

jkellerer
Copy link
Collaborator

@jkellerer jkellerer commented Jun 12, 2023

Aligns the --host filter of "retention" (forget) with "backup" in restic (if host is not set, it defaults to os.Hostname() in restic, but forget has no default matching all hosts if unset)

After this change, when using retention in version 2 configs, all 3 filters path, tags and host are aligned with the backup section if not redefined.

Other changes:

@creativeprojects : before I create tests, what do you think on this?

@creativeprojects
Copy link
Owner

before I create tests, what do you think on this?

Yes this is a good idea 👍🏻

I was also thinking we should fix the inconsistency of retention.after-backup and backup.check-after but that's out of the scope of this MR.

Sorry I'm very busy this week, and the next one as well. We're deploying a new project to production next week 😉

@codecov
Copy link

codecov bot commented Jul 14, 2023

Codecov Report

Patch coverage: 96.83% and project coverage change: +0.09% 🎉

Comparison is base (347501d) 77.39% compared to head (3e0d095) 77.48%.
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #227      +/-   ##
==========================================
+ Coverage   77.39%   77.48%   +0.09%     
==========================================
  Files          93       93              
  Lines       10034    10074      +40     
==========================================
+ Hits         7765     7805      +40     
  Misses       2004     2004              
  Partials      265      265              
Flag Coverage Δ
unittests 77.48% <96.83%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
wrapper.go 85.50% <0.00%> (ø)
config/info_customizer.go 100.00% <100.00%> (ø)
config/profile.go 93.72% <100.00%> (+0.38%) ⬆️

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

@jkellerer jkellerer marked this pull request as ready for review August 5, 2023 14:23
@jkellerer
Copy link
Collaborator Author

@creativeprojects , think the PR is ready for review now. What do you think?

@jkellerer jkellerer added this to the v0.23.0 milestone Aug 5, 2023
Copy link
Owner

@creativeprojects creativeprojects left a comment

Choose a reason for hiding this comment

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

LGTM 😄

BTW we should keep track of all the subtle differences between v1 and v2 so we can make a exhaustive upgrade documentation

Thanks 😉

@jkellerer
Copy link
Collaborator Author

Guess we’ll need to review usages of config.Version when we document differences. This time it is in the reference at least 🙂

@jkellerer jkellerer merged commit ba9297a into creativeprojects:master Aug 7, 2023
@jkellerer jkellerer deleted the enh-retention-v2 branch August 7, 2023 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants