Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: add direct source code to clean up empty folders instead of depending on additional plugin #48
feat: add direct source code to clean up empty folders instead of depending on additional plugin #48
Changes from 5 commits
d30d29b
23313b1
1cc6fe0
21f5849
790e228
ac31a73
a298bc4
c0d2fc8
18882d4
16fe45e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Inline functions are not readable here, it's hard to get the idea from it. Could we move as much as possible outside of the class?
I see there's no
self
usages hereThere 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.
I will move this to a new utils.py file. It's more readable then.
I would still keep it as inline functions since this is a function very specific to the conversion of list to dict, and not used anywhere else.
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.
It might be either a separate function or class method, but differently not a function inside class method
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.
Did some refactoring now. From my point of view it's now ready to be merged.
I would still argue against putting this as a separate function. It would be out of context. If you still disagree, feel free to direclty update this MR.
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.
Let's add a note about
delete_empty_folder
at the endIf you use Artifactory higher then 7.8.1 - it removes empty folders automaticly. We use the rule for some advanced usages, see FAQ section below.
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.
I would leave this up to you to extend this MR with some additional descriptions.