Fix linter errors raised by staticcheck

[rdimitrov]

This is a small PR separating a few fixes for linter errors that would otherwise be raised by golangci-lint/staticcheck in #234.

Signed-off-by: Radoslav Dimitrov [email protected]

[coveralls]

Pull Request Test Coverage Report for Build 2061288962

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 12 of 17 (70.59%) changed or added relevant lines in 1 file are covered.
• 39 unchanged lines in 1 file lost coverage.
• Overall coverage increased (+0.02%) to 71.295%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
local_store.go	12	17	70.59%

Files with Coverage Reduction	New Missed Lines	%
local_store.go	39	78.24%

Totals
Change from base Build 2029695489:	0.02%
Covered Lines:	2290
Relevant Lines:	3212

---

💛 - Coveralls

[joshuagl]

Thanks for working on fixing inter failures. I'd like a second set of eyes on the LocalStore changes.

[trishankatdatadog]

ditto Joshua's comments

[rdimitrov]

@trishankatdatadog @joshuagl - Thanks again! I've addressed the feedback 👍

[trishankatdatadog]

@trishankatdatadog @joshuagl - Thanks again! I've addressed the feedback 👍

I'll wait for Joshua's approval

[rdimitrov]

@trishankatdatadog @joshuagl - The tests caught some side effect of this that it deletes the "targets" folder in case there are no targets at all left in the repository. Do you think that can be a problem? If so, I can update the implementation to skip deleting folders named "targets".

[joshuagl]

@trishankatdatadog @joshuagl - The tests caught some side effect of this that it deletes the "targets" folder in case there are no targets at all left in the repository. Do you think that can be a problem? If so, I can update the implementation to skip deleting folders named "targets".

@trishankatdatadog @ethan-lowman-dd do you anticipate this behaviour causing problems, based on your experience deploying go-tuf?

[trishankatdatadog]

@trishankatdatadog @ethan-lowman-dd do you anticipate this behaviour causing problems, based on your experience deploying go-tuf?

Is it really more dangerous to delete an empty folder than a target file? BTW, what folder is it: is it specific to that target, or is it for all targets?

[rdimitrov]

@trishankatdatadog @ethan-lowman-dd do you anticipate this behaviour causing problems, based on your experience deploying go-tuf?

Is it really more dangerous to delete an empty folder than a target file? BTW, what folder is it: is it specific to that target, or is it for all targets?

Previously, go-tuf ignored deleting folders without any targets in it. Meaning if we have a single target "foo/bar" and we delete it, the "foo" folder will remain even though there's nothing left to be served in it.

Now, this change checks if the parent folder of the target we delete will be left empty, and if yes - deletes it too. In the example above where we delete a target "foo/bar" with location "./repository/targets/foo/bar" - deleting "bar" will delete the "foo" folder too (in case there are no more targets in it). So to your question - it is specific to that target.

The side effect that was caught by the tests is about when we have only 1 target "bar" in the repository with location "./repository/targets/bar" - deleting it will also delete its parent directory, which in this case is the "./repository/targets" folder. We need to decide if this is the expected behavior and if not, what is the correct way to handle that?

[joshuagl]

This change seems reasonable, it's hard to imagine all scenarios where this might cause breakage. I'm inclined to merge and handle error reports if they are filed.

[trishankatdatadog]

This change seems reasonable, it's hard to imagine all scenarios where this might cause breakage. I'm inclined to merge and handle error reports if they are filed.

Approve again as you see fit?

[joshuagl]

Thanks all!