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/01/08 17:16:04 UTC
[GitHub] [sling-org-apache-sling-feature-cpconverter] DominikSuess opened a
new pull request #23: SLING-8973 - fixing runmode handling for repoinit
configuration handling
DominikSuess opened a new pull request #23: SLING-8973 - fixing runmode handling for repoinit configuration handling
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/23
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [sling-org-apache-sling-feature-cpconverter] bosschaert commented
on a change in pull request #23: SLING-8973 - fixing runmode handling for
repoinit configuration handling
Posted by GitBox <gi...@apache.org>.
bosschaert commented on a change in pull request #23: SLING-8973 - fixing runmode handling for repoinit configuration handling
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/23#discussion_r364365501
##########
File path: src/main/java/org/apache/sling/feature/cpconverter/acl/DefaultAclManager.java
##########
@@ -134,7 +134,7 @@ public void addRepoinitExtension(List<VaultPackageAssembler> packageAssemblers,
String text = formatter.toString();
if (!text.isEmpty()) {
- featureManager.addOrAppendRepoInitExtension(text);
+ featureManager.addOrAppendRepoInitExtension(text, null);
Review comment:
Would it make sense to add an extra override to addOrAppendRepoInitExtension() with a single argument that calls the other one with `null` as the second argument? That way we don't have to specify `null` everywhere in the code where we don't need runmodes?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [sling-org-apache-sling-feature-cpconverter] sonarcloud[bot]
commented on issue #23: SLING-8973 - fixing runmode handling for repoinit
configuration handling
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on issue #23: SLING-8973 - fixing runmode handling for repoinit configuration handling
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/23#issuecomment-572188194
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=23&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=23&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=23&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=23&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=23&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=23&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=23&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=23&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=23&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=23&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=23&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=23&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=23&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=23&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-feature-cpconverter&pullRequest=23&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
With regards,
Apache Git Services
[GitHub] [sling-org-apache-sling-feature-cpconverter] cziegeler merged pull
request #23: SLING-8973 - fixing runmode handling for repoinit
configuration handling
Posted by GitBox <gi...@apache.org>.
cziegeler merged pull request #23: SLING-8973 - fixing runmode handling for repoinit configuration handling
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/23
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [sling-org-apache-sling-feature-cpconverter] DominikSuess commented
on a change in pull request #23: SLING-8973 - fixing runmode handling for
repoinit configuration handling
Posted by GitBox <gi...@apache.org>.
DominikSuess commented on a change in pull request #23: SLING-8973 - fixing runmode handling for repoinit configuration handling
URL: https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/23#discussion_r364583305
##########
File path: src/main/java/org/apache/sling/feature/cpconverter/acl/DefaultAclManager.java
##########
@@ -134,7 +134,7 @@ public void addRepoinitExtension(List<VaultPackageAssembler> packageAssemblers,
String text = formatter.toString();
if (!text.isEmpty()) {
- featureManager.addOrAppendRepoInitExtension(text);
+ featureManager.addOrAppendRepoInitExtension(text, null);
Review comment:
I was thinking about that but tbh it makes it easier to miss that you might need to pass a runmode (what made me miss the need to pass it earlier). if you still insist I certainly can add a corresopnding alternative wrapping signature and replace the null calls.
----------------------------------------------------------------
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
With regards,
Apache Git Services