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

Feature Converter doesn't cleanup after itself #28

Merged

Conversation

kaushalmall
Copy link
Contributor

added cleanup method to cleanup directories that the converter create.

FYI @DominikSuess

@DominikSuess
Copy link
Contributor

Despite of the suggestions of @bosschaert I think this PR is improving the situation - we could think of improving it to properly clean up after itself as well but for the issue at hand cleaning the working directories sounds like a good plan.

@cziegeler
Copy link
Contributor

I assume the problem here is a re-run of the tool?
If that is the case, why not delete the directory before running the tool; this way someone just running the tool once does not pay the price of waiting for the directory delete

It would also be good to have a jira issue for this

@bdelacretaz
Copy link
Member

FWIW, I think "synthetic" is spelled wrong in syntethic-content-packages - but maybe that's a reflection of a typo made elsewhere.

@kaushalmall
Copy link
Contributor Author

kaushalmall commented May 28, 2020

Hi @cziegeler @DominikSuess if this is preferred, I can move this code to the corresponding class(es) to delete before the processing starts. Let me know.

why not delete the directory before running the tool;

@DominikSuess do you mind creating an issue for this? Not sure if I have access

It would also be good to have a jira issue for this

Please add that the "synthetic" typo is also fixed as part of this issue :-)

@kaushalmall
Copy link
Contributor Author

@bdelacretaz thanks for catching it, the original class has the same typo, but, I just updated the PR to fix it across the board.

@sonarcloud
Copy link

sonarcloud bot commented May 28, 2020

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

8.3% 8.3% Coverage
0.0% 0.0% Duplication

warning The version of Java (1.8.0_241) you have used to run this analysis is deprecated and we will stop accepting it from October 2020. Please update to at least Java 11.
Read more here

@bosschaert
Copy link
Contributor

do you mind creating an issue for this? Not sure if I have access

Creating a sling issue can be done by anyone. Just create an account and file the issue here: https://issues.apache.org/jira/projects/SLING

@kaushalmall
Copy link
Contributor Author

@kaushalmall
Copy link
Contributor Author

hi @DominikSuess @cziegeler @bdelacretaz @bosschaert any reason for this to not be merged?

@cziegeler cziegeler merged commit 7b57859 into apache:master Jun 26, 2020
@cziegeler
Copy link
Contributor

@kaushalmall Sorry, it's applied now - thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants