You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by GitBox <gi...@apache.org> on 2020/05/14 18:32:55 UTC
[GitHub] [sling-org-apache-sling-feature-cpconverter] kaushalmall opened a new pull request #28: Feature Converter doesn't cleanup after itself
kaushalmall opened a new pull request #28:
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/28
added cleanup method to cleanup directories that the converter create.
FYI @DominikSuess
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-feature-cpconverter] sonarcloud[bot] commented on pull request #28: Feature Converter doesn't cleanup after itself
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #28:
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/28#issuecomment-628945404
SonarCloud Quality Gate failed.
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&metric=new_duplicated_lines_density&view=list)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-feature-cpconverter] DominikSuess commented on a change in pull request #28: Feature Converter doesn't cleanup after itself
Posted by GitBox <gi...@apache.org>.
DominikSuess commented on a change in pull request #28:
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/28#discussion_r431292982
##########
File path: src/main/java/org/apache/sling/feature/cpconverter/cli/ShutDownHook.java
##########
@@ -76,4 +82,15 @@ public void run() {
logger.info("+-----------------------------------------------------+");
}
+ private void cleanUp(){
+ String tmpDir = System.getProperty("java.io.tmpdir");
+ logger.info("Cleaning up tmp directories {}, {}", tmpDir + "sub-content-packages", tmpDir + "syntethic-content-packages" );
Review comment:
@kaushalmall - just a minor thing - tmpdir may not have separator at the end so the error message may actually not really return the right path, creating the the Dir fileobjects ahead and using the path out of those would be cleaner.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-feature-cpconverter] DominikSuess commented on pull request #28: Feature Converter doesn't cleanup after itself
Posted by GitBox <gi...@apache.org>.
DominikSuess commented on pull request #28:
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/28#issuecomment-634790067
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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-feature-cpconverter] cziegeler commented on pull request #28: Feature Converter doesn't cleanup after itself
Posted by GitBox <gi...@apache.org>.
cziegeler commented on pull request #28:
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/28#issuecomment-635139557
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
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-feature-cpconverter] cziegeler commented on pull request #28: Feature Converter doesn't cleanup after itself
Posted by GitBox <gi...@apache.org>.
cziegeler commented on pull request #28:
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/28#issuecomment-649967412
@kaushalmall Sorry, it's applied now - thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-feature-cpconverter] cziegeler merged pull request #28: Feature Converter doesn't cleanup after itself
Posted by GitBox <gi...@apache.org>.
cziegeler merged pull request #28:
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/28
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-feature-cpconverter] kaushalmall commented on a change in pull request #28: Feature Converter doesn't cleanup after itself
Posted by GitBox <gi...@apache.org>.
kaushalmall commented on a change in pull request #28:
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/28#discussion_r431302891
##########
File path: src/main/java/org/apache/sling/feature/cpconverter/cli/ShutDownHook.java
##########
@@ -76,4 +82,15 @@ public void run() {
logger.info("+-----------------------------------------------------+");
}
+ private void cleanUp(){
+ String tmpDir = System.getProperty("java.io.tmpdir");
+ logger.info("Cleaning up tmp directories {}, {}", tmpDir + "sub-content-packages", tmpDir + "syntethic-content-packages" );
+
+ try {
+ FileUtils.deleteDirectory( new File (tmpDir, "syntethic-content-packages") );
+ FileUtils.deleteDirectory( new File(tmpDir, "sub-content-packages") );
+ } catch (IOException e) {
+ logger.error( "Error Deleting {}, {}", tmpDir + "sub-content-packages", tmpDir + "syntethic-content-packages");
+ }
Review comment:
@bosschaert this was conflicting for me, I _was_ going to make them constants, but, the existing classes don't have them as constants, and I decided to follow the same practice for consistency sake. Maybe another issue to track making a common constants class across the board?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-feature-cpconverter] kaushalmall commented on pull request #28: Feature Converter doesn't cleanup after itself
Posted by GitBox <gi...@apache.org>.
kaushalmall commented on pull request #28:
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/28#issuecomment-635434637
@bdelacretaz thanks for catching it, the original class has the same typo, but, I just updated the PR to fix it across the board.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-feature-cpconverter] kaushalmall edited a comment on pull request #28: Feature Converter doesn't cleanup after itself
Posted by GitBox <gi...@apache.org>.
kaushalmall edited a comment on pull request #28:
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/28#issuecomment-635433227
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 :-)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-feature-cpconverter] sonarcloud[bot] commented on pull request #28: Feature Converter doesn't cleanup after itself
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #28:
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/28#issuecomment-635590604
SonarCloud Quality Gate failed.
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0.png' alt='8.3%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&metric=new_coverage&view=list) [8.3% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&metric=new_duplicated_lines_density&view=list)
<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/message_warning.png' alt='warning' width='16' height='16' /> 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](https://sonarcloud.io/documentation/upcoming/)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-feature-cpconverter] bosschaert commented on pull request #28: Feature Converter doesn't cleanup after itself
Posted by GitBox <gi...@apache.org>.
bosschaert commented on pull request #28:
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/28#issuecomment-635914387
> 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
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-feature-cpconverter] bosschaert commented on a change in pull request #28: Feature Converter doesn't cleanup after itself
Posted by GitBox <gi...@apache.org>.
bosschaert commented on a change in pull request #28:
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/28#discussion_r431371846
##########
File path: src/main/java/org/apache/sling/feature/cpconverter/cli/ShutDownHook.java
##########
@@ -76,4 +82,15 @@ public void run() {
logger.info("+-----------------------------------------------------+");
}
+ private void cleanUp(){
+ String tmpDir = System.getProperty("java.io.tmpdir");
+ logger.info("Cleaning up tmp directories {}, {}", tmpDir + "sub-content-packages", tmpDir + "syntethic-content-packages" );
+
+ try {
+ FileUtils.deleteDirectory( new File (tmpDir, "syntethic-content-packages") );
+ FileUtils.deleteDirectory( new File(tmpDir, "sub-content-packages") );
+ } catch (IOException e) {
+ logger.error( "Error Deleting {}, {}", tmpDir + "sub-content-packages", tmpDir + "syntethic-content-packages");
+ }
Review comment:
Ok fair enough. No problem then :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-feature-cpconverter] sonarcloud[bot] removed a comment on pull request #28: Feature Converter doesn't cleanup after itself
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #28:
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/28#issuecomment-628945404
SonarCloud Quality Gate failed.
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=28&metric=new_duplicated_lines_density&view=list)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-feature-cpconverter] kaushalmall commented on pull request #28: Feature Converter doesn't cleanup after itself
Posted by GitBox <gi...@apache.org>.
kaushalmall commented on pull request #28:
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/28#issuecomment-649635530
hi @DominikSuess @cziegeler @bdelacretaz @bosschaert any reason for this to not be merged?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-feature-cpconverter] kaushalmall commented on a change in pull request #28: Feature Converter doesn't cleanup after itself
Posted by GitBox <gi...@apache.org>.
kaushalmall commented on a change in pull request #28:
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/28#discussion_r431489608
##########
File path: src/main/java/org/apache/sling/feature/cpconverter/cli/ShutDownHook.java
##########
@@ -76,4 +82,15 @@ public void run() {
logger.info("+-----------------------------------------------------+");
}
+ private void cleanUp(){
+ String tmpDir = System.getProperty("java.io.tmpdir");
+ logger.info("Cleaning up tmp directories {}, {}", tmpDir + "sub-content-packages", tmpDir + "syntethic-content-packages" );
Review comment:
@DominikSuess hopefully the latest change is better aligned to what you were thinking
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-feature-cpconverter] bosschaert commented on a change in pull request #28: Feature Converter doesn't cleanup after itself
Posted by GitBox <gi...@apache.org>.
bosschaert commented on a change in pull request #28:
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/28#discussion_r431278083
##########
File path: src/main/java/org/apache/sling/feature/cpconverter/cli/ShutDownHook.java
##########
@@ -76,4 +82,15 @@ public void run() {
logger.info("+-----------------------------------------------------+");
}
+ private void cleanUp(){
+ String tmpDir = System.getProperty("java.io.tmpdir");
+ logger.info("Cleaning up tmp directories {}, {}", tmpDir + "sub-content-packages", tmpDir + "syntethic-content-packages" );
+
+ try {
+ FileUtils.deleteDirectory( new File (tmpDir, "syntethic-content-packages") );
+ FileUtils.deleteDirectory( new File(tmpDir, "sub-content-packages") );
+ } catch (IOException e) {
+ logger.error( "Error Deleting {}, {}", tmpDir + "sub-content-packages", tmpDir + "syntethic-content-packages");
+ }
Review comment:
I would make `"sub-content-packages"` and `"syntethic-content-packages"` constants...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-feature-cpconverter] kaushalmall commented on pull request #28: Feature Converter doesn't cleanup after itself
Posted by GitBox <gi...@apache.org>.
kaushalmall commented on pull request #28:
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/28#issuecomment-635433227
>why not delete the directory before running the tool;
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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-feature-cpconverter] kaushalmall commented on pull request #28: Feature Converter doesn't cleanup after itself
Posted by GitBox <gi...@apache.org>.
kaushalmall commented on pull request #28:
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/28#issuecomment-636159564
created https://issues.apache.org/jira/browse/SLING-9490
thanks @bosschaert
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-feature-cpconverter] bdelacretaz commented on pull request #28: Feature Converter doesn't cleanup after itself
Posted by GitBox <gi...@apache.org>.
bdelacretaz commented on pull request #28:
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/28#issuecomment-635154939
FWIW, I think "synthetic" is spelled wrong in `syntethic-content-packages` - but maybe that's a reflection of a typo made elsewhere.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org