Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ZOOKEEPER-3786: Simplify version generation #1310

Closed
wants to merge 1 commit into from
Closed

ZOOKEEPER-3786: Simplify version generation #1310

wants to merge 1 commit into from

Conversation

ctubbsii
Copy link
Member

@ctubbsii ctubbsii commented Apr 7, 2020 •
edited
Loading

Simplify generation of VersionInfoMain.java and Info.java by using
maven-resource-plugin's built-in resource filtering at build time.

This eliminates the need to use VerGen to generate java source files
during the build.

Also make other slight pom improvements:

  1. Remove trailing tab character in Ted's name in pom.xml
  2. Simplify spotbugs skipping in contrib pom.xml
  3. Add m2e configuration for build plugin executions to be ignored by
    Eclipse, for developers (like me) using Eclipse IDE
  4. Format build time in a more international-friendly and less ambiguous
    way (year first, then month, then day, using UTC instead of GMT)

Link to issue: https://issues.apache.org/jira/browse/ZOOKEEPER-3786

eolivelli
eolivelli requested changes Apr 8, 2020
View reviewed changes
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall.
I would keep Eclipse stuff as a separate work

pom.xml Outdated Show resolved Hide resolved
zookeeper-server/src/test/java/org/apache/zookeeper/VerGenTest.java Show resolved Hide resolved
eolivelli
eolivelli reviewed Apr 8, 2020
View reviewed changes
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay about the dropped test.
I misunderstood the meaning of the test

@ctubbsii
ZOOKEEPER-3786: Simplify version generation
7a48e9e 
Simplify generation of VersionInfoMain.java and Info.java by using
maven-resource-plugin's built-in resource filtering at build time.

This eliminates the need to use VerGen to generate java source files
during the build.

Also make other slight pom improvements:
1. Remove trailing tab character in Ted's name in pom.xml
2. Simplify spotbugs skipping in contrib pom.xml
3. Format build time in a more international-friendly and less ambiguous
   way (year first, then month, then day, using UTC instead of GMT)
@ctubbsii
Copy link
Member Author

ctubbsii commented Apr 8, 2020

FWIW, I would have loved to simplify the API for the version information as well (I think the Info interface is a bit more convoluted than necessary). However, I figured avoiding API changes was best, especially since I've seen already the concerns about API breakage and the revision field.

The only thing changed from an end-user perspective is the removal of VerGen (and its unit test), but I see VerGen as a build utility, rather than any kind of API.

symat
symat approved these changes Apr 8, 2020
View reviewed changes
Copy link
Contributor

@symat symat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the idea and the modifications looks good, thanks! :)
I was testing the patch locally using maven 3.5.3.
./bin/zkServer.sh version works too

LGTM, +1

@ctubbsii ctubbsii requested a review from eolivelli April 8, 2020 15:09
@symat
Copy link
Contributor

symat commented Apr 9, 2020

retest maven build

@symat
Copy link
Contributor

symat commented Apr 9, 2020

I'll merge it to master.
@eolivelli what do you think, should we backport to 3.6 / 3.5 ?

@eolivelli
Copy link
Contributor

I think it is not worth to merge to 3.6 and 3.5

@symat
Copy link
Contributor

symat commented Apr 9, 2020

ok, agree. (also it changes the "GMT" string to "UTC", what is not a huge thing, but a change in the version string printed out by bin/zkServer.sh version )

@asfgit asfgit closed this in f9a0803 Apr 9, 2020
@symat
Copy link
Contributor

symat commented Apr 9, 2020

merged to the master branch, thanks @ctubbsii !

@ctubbsii
Copy link
Member Author

ctubbsii commented Apr 9, 2020

No problem. And yes, I agree that it shouldn't be backported. Master-only is fine.

@ctubbsii ctubbsii deleted the use-resource-filtering-for-version-info branch April 9, 2020 18:13
@ctubbsii ctubbsii mentioned this pull request Apr 15, 2020
Closed
eolivelli pushed a commit that referenced this pull request Apr 15, 2020
@ctubbsii @eolivelli
ZOOKEEPER-3738: Fix VerGen when mvngit.commit.id is null
4d1ddef 
Workaround for exec-maven-plugin treating an empty
`<argument>${mvngit.commit.id}</argument>` as null and passing an
incorrect number of arguments to VerGen.

This change allows the revision to be omitted in the command-line args
to VerGen.

This allows the mavanagaiata-maven-plugin to provide the git commit id,
when available, to the VerGen command. This change is superseded in
ZooKeeper 3.7.0 and later by ZOOKEEPER-3786
(#1310), which simplifies
generating the version information and removes VerGen.

Author: Christopher Tubbs <ctubbsii@apache.org>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <szalay.beko.mate@gmail.com>

Closes #1321 from ctubbsii/fix-vergen-for-36-mavanagaiata
eolivelli pushed a commit that referenced this pull request Apr 15, 2020
@ctubbsii @eolivelli
ZOOKEEPER-3738: Fix VerGen when mvngit.commit.id is null
a33ab47 
Workaround for exec-maven-plugin treating an empty
`<argument>${mvngit.commit.id}</argument>` as null and passing an
incorrect number of arguments to VerGen.

This change allows the revision to be omitted in the command-line args
to VerGen.

This allows the mavanagaiata-maven-plugin to provide the git commit id,
when available, to the VerGen command. This change is superseded in
ZooKeeper 3.7.0 and later by ZOOKEEPER-3786
(#1310), which simplifies
generating the version information and removes VerGen.

Author: Christopher Tubbs <ctubbsii@apache.org>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <szalay.beko.mate@gmail.com>

Closes #1321 from ctubbsii/fix-vergen-for-36-mavanagaiata
stickyhipp pushed a commit to stickyhipp/zookeeper that referenced this pull request Aug 19, 2020
@ctubbsii @stickyhipp
ZOOKEEPER-3786: Simplify version generation
47638d8 
Simplify generation of VersionInfoMain.java and Info.java by using
maven-resource-plugin's built-in resource filtering at build time.

This eliminates the need to use VerGen to generate java source files
during the build.

Also make other slight pom improvements:
1. Remove trailing tab character in Ted's name in pom.xml
2. Simplify spotbugs skipping in contrib pom.xml
3. Add m2e configuration for build plugin executions to be ignored by
   Eclipse, for developers (like me) using Eclipse IDE
4. Format build time in a more international-friendly and less ambiguous
   way (year first, then month, then day, using UTC instead of GMT)

Link to issue: https://issues.apache.org/jira/browse/ZOOKEEPER-3786

Author: Christopher Tubbs <ctubbsii@apache.org>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org>

Closes apache#1310 from ctubbsii/use-resource-filtering-for-version-info
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Aug 31, 2022
@ctubbsii @RokLenarcic
ZOOKEEPER-3786: Simplify version generation
45e252b 
Simplify generation of VersionInfoMain.java and Info.java by using
maven-resource-plugin's built-in resource filtering at build time.

This eliminates the need to use VerGen to generate java source files
during the build.

Also make other slight pom improvements:
1. Remove trailing tab character in Ted's name in pom.xml
2. Simplify spotbugs skipping in contrib pom.xml
3. Add m2e configuration for build plugin executions to be ignored by
   Eclipse, for developers (like me) using Eclipse IDE
4. Format build time in a more international-friendly and less ambiguous
   way (year first, then month, then day, using UTC instead of GMT)

Link to issue: https://issues.apache.org/jira/browse/ZOOKEEPER-3786

Author: Christopher Tubbs <ctubbsii@apache.org>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org>

Closes apache#1310 from ctubbsii/use-resource-filtering-for-version-info
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Aug 31, 2022
@ctubbsii @RokLenarcic
ZOOKEEPER-3786: Simplify version generation
d781fd2 
Simplify generation of VersionInfoMain.java and Info.java by using
maven-resource-plugin's built-in resource filtering at build time.

This eliminates the need to use VerGen to generate java source files
during the build.

Also make other slight pom improvements:
1. Remove trailing tab character in Ted's name in pom.xml
2. Simplify spotbugs skipping in contrib pom.xml
3. Add m2e configuration for build plugin executions to be ignored by
   Eclipse, for developers (like me) using Eclipse IDE
4. Format build time in a more international-friendly and less ambiguous
   way (year first, then month, then day, using UTC instead of GMT)

Link to issue: https://issues.apache.org/jira/browse/ZOOKEEPER-3786

Author: Christopher Tubbs <ctubbsii@apache.org>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org>

Closes apache#1310 from ctubbsii/use-resource-filtering-for-version-info
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Aug 31, 2022
@ctubbsii @RokLenarcic
ZOOKEEPER-3786: Simplify version generation
b26f485 
Simplify generation of VersionInfoMain.java and Info.java by using
maven-resource-plugin's built-in resource filtering at build time.

This eliminates the need to use VerGen to generate java source files
during the build.

Also make other slight pom improvements:
1. Remove trailing tab character in Ted's name in pom.xml
2. Simplify spotbugs skipping in contrib pom.xml
3. Add m2e configuration for build plugin executions to be ignored by
   Eclipse, for developers (like me) using Eclipse IDE
4. Format build time in a more international-friendly and less ambiguous
   way (year first, then month, then day, using UTC instead of GMT)

Link to issue: https://issues.apache.org/jira/browse/ZOOKEEPER-3786

Author: Christopher Tubbs <ctubbsii@apache.org>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org>

Closes apache#1310 from ctubbsii/use-resource-filtering-for-version-info
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Sep 3, 2022
@ctubbsii @RokLenarcic
ZOOKEEPER-3786: Simplify version generation
4a41e9f 
Simplify generation of VersionInfoMain.java and Info.java by using
maven-resource-plugin's built-in resource filtering at build time.

This eliminates the need to use VerGen to generate java source files
during the build.

Also make other slight pom improvements:
1. Remove trailing tab character in Ted's name in pom.xml
2. Simplify spotbugs skipping in contrib pom.xml
3. Add m2e configuration for build plugin executions to be ignored by
   Eclipse, for developers (like me) using Eclipse IDE
4. Format build time in a more international-friendly and less ambiguous
   way (year first, then month, then day, using UTC instead of GMT)

Link to issue: https://issues.apache.org/jira/browse/ZOOKEEPER-3786

Author: Christopher Tubbs <ctubbsii@apache.org>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org>

Closes apache#1310 from ctubbsii/use-resource-filtering-for-version-info
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eolivelli eolivelli eolivelli approved these changes

@symat symat symat approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ctubbsii @symat @eolivelli