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

Added prune as supported, schedulable command #24

Merged

Conversation

jkellerer
Copy link
Collaborator

@jkellerer jkellerer commented Dec 20, 2020

Fixes #22

Implementation Notes:

  • profiles_test & show was checked in with forced CRLF (other files were not).
    The diff (without ignored whitespace) is larger than it should. Needs to be clarified if CRLF should be forced.
  • I've consolidated schedule options since Scheduling "retention" reads policy from "forget" #23 may need one more schedulable section.
  • All schedulable sections print with one newline in show. Needs to be clarified if this should be fixed.

Marked as WIP since I only tested it via profiles_test and on my mac so far.

Tested in regular backup schedule and via unit and manual tests.

@codecov
Copy link

codecov bot commented Dec 20, 2020

Codecov Report

Merging #24 (63bbd5c) into master (6544052) will increase coverage by 1.04%.
The diff coverage is 91.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #24      +/-   ##
==========================================
+ Coverage   42.83%   43.87%   +1.04%     
==========================================
  Files          43       43              
  Lines        2762     2767       +5     
==========================================
+ Hits         1183     1214      +31     
+ Misses       1488     1462      -26     
  Partials       91       91              
Impacted Files Coverage Δ
schedule/schedule.go 0.00% <ø> (ø)
config/profile.go 54.12% <89.65%> (+23.85%) ⬆️
config/show.go 80.30% <100.00%> (+1.61%) ⬆️

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 6544052...63bbd5c. Read the comment docs.

@jkellerer
Copy link
Collaborator Author

Self note: resticprofile show returns invalid content with this change. Needs to be checked.

@jkellerer jkellerer changed the title WIP: Added prune as supported, schedulable command Added prune as supported, schedulable command Dec 22, 2020
@jkellerer jkellerer marked this pull request as ready for review December 22, 2020 12:55
@jkellerer jkellerer force-pushed the ft-allow-schedule-prune branch from e9709b5 to cdc41e4 Compare December 22, 2020 13:00
@jkellerer
Copy link
Collaborator Author

Please note that the fix for show adds one newline. I‘d fix this before merge, however would like to get feedback first.

@creativeprojects
Copy link
Owner

Sure, I have been a bit busy, sorry about that.
I'll have a look at it in the next few days 👍🏻

Thanks

@creativeprojects
Copy link
Owner

I see the two annoying files with CRLF ending, it's because sometimes I fix things on my work laptop.
To make things easier I can change them to LF and then maybe we can rebase the PR?

Sorry about that

@jkellerer
Copy link
Collaborator Author

Yes sure, that would be great.

@creativeprojects
Copy link
Owner

Can you try a rebase now?

Handles embedded structs that are annotated with `mapstructure:",squash"`
@jkellerer jkellerer force-pushed the ft-allow-schedule-prune branch from cdc41e4 to 63bbd5c Compare January 8, 2021 14:46
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 8, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@creativeprojects
Copy link
Owner

Cool, thanks

I like the way you refactored the scheduling config into ScheduleSection, nice one 👍🏻

I understand now why you had to change the code of the show command, I have to admit I was a bit puzzled over the holidays when I glanced at the PR message 😆

I'm looking at the newline issue; I realise the code of the show command is rather messy. I remember I was learning go reflection on this one...

@creativeprojects
Copy link
Owner

It's ok for the show command, I need to work on it later.

Thanks fo the PR 👍🏻

@creativeprojects creativeprojects merged commit ec02310 into creativeprojects:master Jan 8, 2021
@jkellerer jkellerer deleted the ft-allow-schedule-prune branch January 9, 2021 12:16
@jkellerer
Copy link
Collaborator Author

Thanks a lot for reviewing and merging!

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.

Allow to schedule "prune"
2 participants