You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by GitBox <gi...@apache.org> on 2020/02/25 18:49:15 UTC
[GitHub] [zookeeper] ctubbsii opened a new pull request #1268:
ZOOKEEPER-3738: Use mavanagaiata for git commit id
ctubbsii opened a new pull request #1268: ZOOKEEPER-3738: Use mavanagaiata for git commit id
URL: https://github.com/apache/zookeeper/pull/1268
Use mavanagaiata plugin to get git commit id for VerGen instead of
properties-maven-plugin (which is broken in some environments).
Also add the commit to the jar manifests for easy reference when given a
jar of unknown origin (especially useful for SNAPSHOT builds).
----------------------------------------------------------------
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] [zookeeper] asfgit closed pull request #1268: ZOOKEEPER-3738: Use
mavanagaiata for git commit id
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #1268: ZOOKEEPER-3738: Use mavanagaiata for git commit id
URL: https://github.com/apache/zookeeper/pull/1268
----------------------------------------------------------------
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] [zookeeper] ctubbsii commented on issue #1268: ZOOKEEPER-3738: Use
mavanagaiata for git commit id
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #1268: ZOOKEEPER-3738: Use mavanagaiata for git commit id
URL: https://github.com/apache/zookeeper/pull/1268#issuecomment-592689866
@nkalmar No problem. I'm curious. What is the typical process for merging stuff in?
----------------------------------------------------------------
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] [zookeeper] ctubbsii edited a comment on issue #1268:
ZOOKEEPER-3738: Use mavanagaiata for git commit id
Posted by GitBox <gi...@apache.org>.
ctubbsii edited a comment on issue #1268: ZOOKEEPER-3738: Use mavanagaiata for git commit id
URL: https://github.com/apache/zookeeper/pull/1268#issuecomment-591078818
> We should also drop VerGen stuff and simply use maven filtering to create a properties file to be read from class path at runtime.
I was also taking a look at the VerGen class and thinking about improving it a bit. I think it's useful to have hard-coded class, rather than read from class path... because class path pollution can mislead the code logic.
So, instead, I suggest doing what I did a very long time ago for Accumulo, and use mvn resource filtering to inject the version information from the POM into the java code, and then use the build-helper-maven-plugin to put that generated code onto the compile-time class path. It's serves a similar purpose as the current VerGen stuff, but I think the implementation is a bit nicer (certainly smaller, and I think more maintainable):
https://github.com/apache/accumulo/blob/master/core/pom.xml#L266-L305
----------------------------------------------------------------
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] [zookeeper] nkalmar commented on issue #1268: ZOOKEEPER-3738: Use
mavanagaiata for git commit id
Posted by GitBox <gi...@apache.org>.
nkalmar commented on issue #1268: ZOOKEEPER-3738: Use mavanagaiata for git commit id
URL: https://github.com/apache/zookeeper/pull/1268#issuecomment-593480253
Merged to master and 3.6, thanks @ctubbsii
----------------------------------------------------------------
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] [zookeeper] eolivelli commented on issue #1268: ZOOKEEPER-3738: Use
mavanagaiata for git commit id
Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #1268: ZOOKEEPER-3738: Use mavanagaiata for git commit id
URL: https://github.com/apache/zookeeper/pull/1268#issuecomment-592903657
We need at least two approvals from committers.
In this case this patch is good to go (me and Norbert).
I will merge as soon as possible
We are following review then commit
----------------------------------------------------------------
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] [zookeeper] nkalmar commented on issue #1268: ZOOKEEPER-3738: Use
mavanagaiata for git commit id
Posted by GitBox <gi...@apache.org>.
nkalmar commented on issue #1268: ZOOKEEPER-3738: Use mavanagaiata for git commit id
URL: https://github.com/apache/zookeeper/pull/1268#issuecomment-593332943
I'll merge it now. As Enrico said 2 committer +1. On some cases 1 is enough, as ZooKeeper is not the most active component (but fortunately getting more and more active members lately).
I didn't merge last time because the patch was only a few (7 I think) hours old, and we had an agreement during the maven migration that we'll let a PR related to maven unmerged for 3 days minimum. This is not the case anymore, but habits die hard :)
----------------------------------------------------------------
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] [zookeeper] ctubbsii commented on issue #1268: ZOOKEEPER-3738: Use
mavanagaiata for git commit id
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #1268: ZOOKEEPER-3738: Use mavanagaiata for git commit id
URL: https://github.com/apache/zookeeper/pull/1268#issuecomment-591078818
> We should also drop VerGen stuff and simply use maven filtering to create a properties file to be read from class path at runtime.
I was also taking a look at the VerGen class and thinking about improving it a bit. I think it's useful to have hard-coded class, rather than read from class path... because class path pollution can mislead the code logic.
So, instead, I suggest doing what I did a very long time ago for Accumulo, and use mvn resource filtering to inject the version information from the POM into the java code, and then use the build-helper-maven-plugin to put that generated code onto the compile-time class path. It's serves a similar purpose as the current VerGen stuff, but I think the implementation is a bit nicer (certainly smaller):
https://github.com/apache/accumulo/blob/master/core/pom.xml#L266-L305
----------------------------------------------------------------
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] [zookeeper] ctubbsii commented on issue #1268: ZOOKEEPER-3738: Use
mavanagaiata for git commit id
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #1268: ZOOKEEPER-3738: Use mavanagaiata for git commit id
URL: https://github.com/apache/zookeeper/pull/1268#issuecomment-593518736
Cool. Thanks! I'm just glad you're on Maven now. It's much easier to understand the build now, I think.
----------------------------------------------------------------
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