[GitHub] [mina-sshd] tomaswolf commented on a diff in pull request #343: Simplify maven-bundle-plugin configuration

[via GitHub]

tomaswolf commented on code in PR #343:
URL: https://github.com/apache/mina-sshd/pull/343#discussion_r1145284636


##########
pom.xml:
##########
@@ -1445,31 +1445,20 @@
                 <inherited>true</inherited>
                 <extensions>true</extensions>
                 <executions>
-                    <execution>
-                        <id>versions</id>
-                        <phase>validate</phase>
-                        <goals>
-                            <goal>cleanVersions</goal>
-                        </goals>
-                        <configuration>
-                            <versions>
-                                
<sshd.osgi.version.clean>${project.version}</sshd.osgi.version.clean>
-                            </versions>
-                        </configuration>
-                    </execution>
                     <execution>
                         <id>bundle-manifest</id>
                         <phase>process-classes</phase>
                         <goals>
                             <goal>manifest</goal>
                         </goals>
                         <configuration>
+                            
<supportIncrementalBuild>true</supportIncrementalBuild>
                             <instructions>
                                 <Import-Package><![CDATA[
-                                  
org.apache.sshd*;version="[$(version;==;${sshd.osgi.version.clean}),$(version;=+;${sshd.osgi.version.clean}))",
+                                  
org.apache.sshd*;version="${range;[==,=+);${maven_version;${project.version}}}",

Review Comment:
   Nice.
   
   However: I just noticed that here (and in the slf4j line below) we have an 
unholy mixture of bnd macro references and maven property references. What 
happens if someone defines a maven property named "range"?
   
   bnd allows other brackets to be used. Can we please use `$(...)` for bnd, 
and reserve `${...}` for maven? Or use `$${macro...}` for bnd references; see 
[the maven-bundle-plugin 
documentation](https://felix.apache.org/documentation/_attachments/components/bundle-plugin/manifest-mojo.html#instructions).
   
   Only tangentially related to this change: I wonder if ==,=+ is good. Apache 
MINA sshd has an unfortunate habit of breaking API in minor versions, so it 
might actually be safer to require exact matches, and not a range at all... 
something like `$(version;===;$(maven_version;${project.version}))`. Or at 
least use a range [===;=+).



-- 
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...@mina.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org