Re: [PR] SLING-12089 Jenkins: Generate javadocs to spot illegal javadoc markup in sources [sling-tooling-jenkins]

2023-10-16 Thread via GitHub


stefanseifert merged PR #19:
URL: https://github.com/apache/sling-tooling-jenkins/pull/19


-- 
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.

To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] SLING-12089 Jenkins: Generate javadocs to spot illegal javadoc markup in sources [sling-tooling-jenkins]

2023-10-13 Thread via GitHub


kwin commented on code in PR #19:
URL: 
https://github.com/apache/sling-tooling-jenkins/pull/19#discussion_r1358126094


##
vars/slingOsgiBundleBuild.groovy:
##
@@ -189,6 +189,8 @@ def defineStage(def globalConfig, def jobConfig, def 
jdkVersion, def operatingSy
 }
 // calculate coverage with jacoco (for subsequent evaluation by 
SonarQube)
 additionalMavenParams = "${additionalMavenParams} -Pjacoco-report"
+// generate javadocs to detect illegal javadoc markup in sources
+additionalMavenParams = "javadoc:javadoc ${additionalMavenParams}"

Review Comment:
   Ok, let's track this separately.



-- 
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.

To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] SLING-12089 Jenkins: Generate javadocs to spot illegal javadoc markup in sources [sling-tooling-jenkins]

2023-10-13 Thread via GitHub


stefanseifert commented on code in PR #19:
URL: 
https://github.com/apache/sling-tooling-jenkins/pull/19#discussion_r1358063124


##
vars/slingOsgiBundleBuild.groovy:
##
@@ -189,6 +189,8 @@ def defineStage(def globalConfig, def jobConfig, def 
jdkVersion, def operatingSy
 }
 // calculate coverage with jacoco (for subsequent evaluation by 
SonarQube)
 additionalMavenParams = "${additionalMavenParams} -Pjacoco-report"
+// generate javadocs to detect illegal javadoc markup in sources
+additionalMavenParams = "javadoc:javadoc ${additionalMavenParams}"

Review Comment:
   i thought about this as well, but the variable goal is used in other placed 
to distinguish between different code paths, and i do not wanted to meddle with 
this as it might lead to unexpected results if more conditions like this are 
added later.
   
   the current solution mixing in the additional goal in the 
additionalMavenParams is also not optimal, but should have not sideffects.
   
   maybe the whole script should be refactored a bit to make it easier to 
inject additional goals without risking the condition checks based on the 
primary goal.



-- 
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.

To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] SLING-12089 Jenkins: Generate javadocs to spot illegal javadoc markup in sources [sling-tooling-jenkins]

2023-10-13 Thread via GitHub


kwin commented on code in PR #19:
URL: 
https://github.com/apache/sling-tooling-jenkins/pull/19#discussion_r1358030566


##
vars/slingOsgiBundleBuild.groovy:
##
@@ -189,6 +189,8 @@ def defineStage(def globalConfig, def jobConfig, def 
jdkVersion, def operatingSy
 }
 // calculate coverage with jacoco (for subsequent evaluation by 
SonarQube)
 additionalMavenParams = "${additionalMavenParams} -Pjacoco-report"
+// generate javadocs to detect illegal javadoc markup in sources
+additionalMavenParams = "javadoc:javadoc ${additionalMavenParams}"

Review Comment:
   can you extend variable `goal` instead? That way the order is implicitly 
correct and we have a better separation between goal and other parameters.



-- 
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.

To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] SLING-12089 Jenkins: Generate javadocs to spot illegal javadoc markup in sources [sling-tooling-jenkins]

2023-10-13 Thread via GitHub


stefanseifert commented on PR #19:
URL: 
https://github.com/apache/sling-tooling-jenkins/pull/19#issuecomment-1761135700

   i've testes this with an experimental PR and it seems to work fine:
   
https://github.com/apache/sling-org-apache-sling-testing-osgi-mock/pull/32/commits


-- 
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.

To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] SLING-12089 Jenkins: Generate javadocs to spot illegal javadoc markup in sources [sling-tooling-jenkins]

2023-10-11 Thread via GitHub


kwin commented on PR #19:
URL: 
https://github.com/apache/sling-tooling-jenkins/pull/19#issuecomment-1757457850

   > this is a very simplistic approach. how is it possible to test this before 
actually merging it?
   
   Testing is described in 
https://github.com/apache/sling-tooling-jenkins#test-changes


-- 
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.

To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] SLING-12089 Jenkins: Generate javadocs to spot illegal javadoc markup in sources [sling-tooling-jenkins]

2023-10-11 Thread via GitHub


stefanseifert commented on PR #19:
URL: 
https://github.com/apache/sling-tooling-jenkins/pull/19#issuecomment-1757451916

   this is a very simplistic approach.
   how is it possible to test this before actually merging it?


-- 
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.

To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org