-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
forget: group-by-tags-only #1196
Conversation
Travis fails because the manpage is not up-to-date in the PR. In your branch, could you please call |
And it needs a rebase, just like the other PR. Let us know if you need help with the rebase. |
d42e815
to
e4a5cdc
Compare
Can I suggest to resolve this in a different way? Instead of adding |
@middelink that is an excellent idea, thank you very much! We can just fill the JSON key according to what's present in @mungomat Do you think you can tackle this? Or shall I do that? |
The CI test fail because |
cmd/restic/cmd_forget.go
Outdated
var GroupByHost bool | ||
var GroupByPath bool | ||
|
||
GroupByTag = strings.Contains( opts.GroupBy, "tag" ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is very vague. I'd go for splitting opts.GroupBy
by ,
, then iterate over the results, and use a switch
statement that reacts on the strings tag
, host
and path
and returns an error if the string is unknown.
What do you think?
69efa47
to
00527c0
Compare
00527c0
to
8f9ef44
Compare
Codecov Report
@@ Coverage Diff @@
## master #1196 +/- ##
==========================================
- Coverage 52.9% 47.12% -5.79%
==========================================
Files 123 126 +3
Lines 12170 12230 +60
==========================================
- Hits 6439 5763 -676
- Misses 4975 5765 +790
+ Partials 756 702 -54
Continue to review full report at Codecov.
|
Good work, thanks! |
This is not complete - what if "restic forget --group-by-tags --group-by-tags-only" is given?