You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by johnament <gi...@git.apache.org> on 2015/12/21 23:18:29 UTC

[GitHub] activemq-artemis pull request: ARTEMIS-326 Quote logging configura...

GitHub user johnament opened a pull request:

    https://github.com/apache/activemq-artemis/pull/282

    ARTEMIS-326 Quote logging configuration to account for spaces.  Remov…

    …ed unnecessary duplicate declarations of variable in all poms.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/johnament/activemq-artemis ARTEMIS-326

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/activemq-artemis/pull/282.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #282
    
----
commit 27f629b3c19b9abbc8b3f9035b2b5394b559939a
Author: John D. Ament <jo...@apache.org>
Date:   2015-12-21T22:19:48Z

    ARTEMIS-326 Quote logging configuration to account for spaces.  Removed unnecessary duplicate declarations of variable in all poms.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request: ARTEMIS-326 Quote logging configura...

Posted by johnament <gi...@git.apache.org>.
Github user johnament commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/282#discussion_r48204956
  
    --- Diff: pom.xml ---
    @@ -115,11 +115,11 @@
           -->
     
           <activemq-surefire-argline>-Djava.util.logging.manager=org.jboss.logmanager.LogManager
    -         -Dlogging.configuration=file:${activemq.basedir}/tests/config/logging.properties
    -         -Djava.library.path=${activemq.basedir}/artemis-native/bin/ -Djgroups.bind_addr=localhost -Dorg.apache.activemq.artemis.api.core.UDPBroadcastEndpointFactory.localBindAddress=localhost
    +         -Dlogging.configuration='file:${activemq.basedir}/tests/config/logging.properties'
    +         -Djava.library.path='${activemq.basedir}/artemis-native/bin/' -Djgroups.bind_addr=localhost -Dorg.apache.activemq.artemis.api.core.UDPBroadcastEndpointFactory.localBindAddress=localhost
              -Djava.net.preferIPv4Stack=true
           </activemq-surefire-argline>
    -      <activemq.basedir>${project.basedir}</activemq.basedir>
    +      <activemq.basedir>${basedir}</activemq.basedir>
    --- End diff --
    
    Agreed.  However, when I didn't use the consolidated variable, and instead let it recompute each time, I had to quote on each declaration.  It wasn't pretty.  I can switch to that if 3.0 compatibility is a must.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request: ARTEMIS-326 Quote logging configura...

Posted by johnament <gi...@git.apache.org>.
Github user johnament commented on the pull request:

    https://github.com/apache/activemq-artemis/pull/282#issuecomment-166449487
  
    Ok, it seems like it was a jenkins config issue.
    
    I changed your config to explicitly point to 3.2.5 (which is the minimum version required as per your pom).  https://builds.apache.org/job/ActiveMQ-Artemis-PR-Build/jobConfigHistory/
    
    Let me know if you want me to revert it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request: ARTEMIS-326 Quote logging configura...

Posted by johnament <gi...@git.apache.org>.
Github user johnament commented on the pull request:

    https://github.com/apache/activemq-artemis/pull/282#issuecomment-166446355
  
    Lets see if this fixes it.  I'm on 3.3.9, so I'm wondering if it may be a difference in variable resolution.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request: ARTEMIS-326 Quote logging configura...

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:

    https://github.com/apache/activemq-artemis/pull/282#issuecomment-166473833
  
    These changes are on the right track, however the build is now broken. For example examples are not building, the distribution is not including the examples.. and mvn verify from the examples doesn't work.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request: ARTEMIS-326 Quote logging configura...

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:

    https://github.com/apache/activemq-artemis/pull/282#issuecomment-166443987
  
    checkstyle is not finding the resource.. that variable is not working.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request: ARTEMIS-326 Quote logging configura...

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:

    https://github.com/apache/activemq-artemis/pull/282#issuecomment-166473974
  
    On a good note Idea integration still works.. the issue now is on the examples


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request: ARTEMIS-326 Quote logging configura...

Posted by johnament <gi...@git.apache.org>.
Github user johnament commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/282#discussion_r48199519
  
    --- Diff: pom.xml ---
    @@ -115,11 +115,11 @@
           -->
     
           <activemq-surefire-argline>-Djava.util.logging.manager=org.jboss.logmanager.LogManager
    -         -Dlogging.configuration=file:${activemq.basedir}/tests/config/logging.properties
    -         -Djava.library.path=${activemq.basedir}/artemis-native/bin/ -Djgroups.bind_addr=localhost -Dorg.apache.activemq.artemis.api.core.UDPBroadcastEndpointFactory.localBindAddress=localhost
    +         -Dlogging.configuration='file:${activemq.basedir}/tests/config/logging.properties'
    +         -Djava.library.path='${activemq.basedir}/artemis-native/bin/' -Djgroups.bind_addr=localhost -Dorg.apache.activemq.artemis.api.core.UDPBroadcastEndpointFactory.localBindAddress=localhost
              -Djava.net.preferIPv4Stack=true
           </activemq-surefire-argline>
    -      <activemq.basedir>${project.basedir}</activemq.basedir>
    +      <activemq.basedir>${basedir}</activemq.basedir>
    --- End diff --
    
    `${basedir}` represents the directory that this pom is in, absolutely, without any doubts.  No reason to redeclare it in every pom.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request: ARTEMIS-326 Quote logging configura...

Posted by johnament <gi...@git.apache.org>.
Github user johnament commented on the pull request:

    https://github.com/apache/activemq-artemis/pull/282#issuecomment-166477108
  
    I'm going to decline this PR.  Looks like my initial issue with single quotes was user error and that's working fine.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request: ARTEMIS-326 Quote logging configura...

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:

    https://github.com/apache/activemq-artemis/pull/282#issuecomment-166456575
  
    I wouldn't have a problem on bumping the minimal requirement for maven. the only reason nobody did it is because it wasn't needed until now. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request: ARTEMIS-326 Quote logging configura...

Posted by johnament <gi...@git.apache.org>.
Github user johnament closed the pull request at:

    https://github.com/apache/activemq-artemis/pull/282


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request: ARTEMIS-326 Quote logging configura...

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/282#discussion_r48204622
  
    --- Diff: pom.xml ---
    @@ -115,11 +115,11 @@
           -->
     
           <activemq-surefire-argline>-Djava.util.logging.manager=org.jboss.logmanager.LogManager
    -         -Dlogging.configuration=file:${activemq.basedir}/tests/config/logging.properties
    -         -Djava.library.path=${activemq.basedir}/artemis-native/bin/ -Djgroups.bind_addr=localhost -Dorg.apache.activemq.artemis.api.core.UDPBroadcastEndpointFactory.localBindAddress=localhost
    +         -Dlogging.configuration='file:${activemq.basedir}/tests/config/logging.properties'
    +         -Djava.library.path='${activemq.basedir}/artemis-native/bin/' -Djgroups.bind_addr=localhost -Dorg.apache.activemq.artemis.api.core.UDPBroadcastEndpointFactory.localBindAddress=localhost
              -Djava.net.preferIPv4Stack=true
           </activemq-surefire-argline>
    -      <activemq.basedir>${project.basedir}</activemq.basedir>
    +      <activemq.basedir>${basedir}</activemq.basedir>
    --- End diff --
    
    There are some sub projects that will need to find files in reference to the base-dir... (e.g. the native library)... and we had issues with basedir in the past. Maybe it's a versioning thing?
    
    
    Also this is used on Idea settings. When you run tests. it will pickup the location from this property and it is a bit tricky to get it working. I would need to test and see if it's ok. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request: ARTEMIS-326 Quote logging configura...

Posted by johnament <gi...@git.apache.org>.
Github user johnament commented on the pull request:

    https://github.com/apache/activemq-artemis/pull/282#issuecomment-166453979
  
    Errr ok.  I just looked again and must have been seeing things.  Let me know what you'd like to do.  3.0.0 is pretty old as of now, should that really be the min? I can email on the dev list to get more input.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---