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.

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

Display Git commit info on command line #138

Merged
merged 1 commit into from
Mar 4, 2014

Conversation

smondet
Copy link
Contributor

@smondet smondet commented Feb 23, 2014

This is a first attempt at #41
(a “pick-me-up”).
To dump useful information while reporting bugs.

 $ java -jar adam-cli/target/adam-0.6.1-SNAPSHOT.jar buildinformation
Build information:
git.commit.id.abbrev: b476864
git.commit.message.full: Do code clean-up (BuildInformation)

git.commit.id: b476864265ee83660ea3cdb116241fb853720294
git.commit.message.short: Do code clean-up (BuildInformation)
git.commit.id.describe: adam-parent-0.6.0-69-gb476864
git.branch: git_properties
git.commit.time: 23.02.2014 @ 14:43:05 EST
git.remote.origin.url: git@github.com:bigdatagenomics/adam
git.build.time: 23.02.2014 @ 14:45:25 EST

It uses https://github.com/ktoso/maven-git-commit-id-plugin

@carlyeks
Copy link
Member

Thanks for the great contribution, Sebastien!

I think we probably want this integrated in the adam-core (possibly in addition to adam-cli). A few use cases that will require this:

  • If we annotate the Parquet files that are created, this will need to be accessed in the adam-core module
  • If we are calling from a different repository, it would be good to know what the version of the library is that we are using

It would be nice to embed the version, and associated checking, in a method as well.

Also, could you rebase this down to a single commit? It's how we have been accepting new features.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@massie
Copy link
Member

massie commented Feb 23, 2014

Jenkins, please add to whitelist and test.

@AmplabJenkins
Copy link

One or more automated tests failed
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/ADAM-prb/153/

@massie
Copy link
Member

massie commented Feb 23, 2014

Jenkins, retest this.

import java.util.Properties

object BuildInformation extends AdamCommandCompanion {
val commandName: String = "buildinformation"
Copy link
Member

Choose a reason for hiding this comment

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

maybe "buildinfo"? It would be shorter to type.

@massie
Copy link
Member

massie commented Feb 23, 2014

Overall, this looks great. Thanks for the contribution, Sebastien!

Jenkins is having some issues running the tests but it's not related to your code. I'm tracking that down.

Just a few small nits about naming and exposing this info in the core module.

@smondet
Copy link
Contributor Author

smondet commented Feb 24, 2014

OK I will collapse the commits, change to buildinfo, and put the base code in a class in adam-core!

Thanks for the feedback

@tdanford
Copy link
Contributor

Don't forget to rebase off of master too, before we merge.

@massie
Copy link
Member

massie commented Feb 24, 2014

Jenkins, retest this.

@massie
Copy link
Member

massie commented Feb 24, 2014

Jenkins, retest this please.

@AmplabJenkins
Copy link

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/ADAM-prb/157/

@massie
Copy link
Member

massie commented Feb 25, 2014

All the tests are passing. This looks good. Once you make your changes, just push them to your git_properties branch to update this pull request.

Looking forward to getting this into ADAM. Thanks for the contribution, Sebastien.

@smondet
Copy link
Contributor Author

smondet commented Mar 2, 2014

Hi

Sorry for the delay, but here it is rebased :)

The object edu.berkeley.cs.amplab.adam.core.util.BuildInformation has two methods asMap and asString.

And the adam-cli command name is buildinfo.

Cheers

}

class BuildInformationArgs extends Args4jBase with ParquetArgs {
// @Args4jOption(required = false, name = "-output", usage = "Output to a file instead of <stdout>")
Copy link
Contributor

Choose a reason for hiding this comment

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

The arguments are commented out here - are they needed? Do we need BuildInformationArgs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was seeing command line modules a bit like templates easy to fill-in in the future.

Bu if you think it's cleaner, I can try to remove it.

Copy link
Member

Choose a reason for hiding this comment

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

@arahuja I believe that a BuildInformationArgs class is required to be passed as a type parameter to the AdamSparkCommand class. However, this new functionality just inherits from an AdamCommand, so I don't think the argument class is needed. @smondet, I think it would be cleaner to remove the BuildInformationArgs class, as it is unused.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fnothaft, OK done, and rebased :)

@AmplabJenkins
Copy link

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/ADAM-prb/187/

@fnothaft fnothaft assigned tdanford and unassigned tdanford Mar 3, 2014
This is a first attempt at bigdatagenomics#41.

- adam-core: Add git-commit plugin to POM file. The file is generated
is `adam-core/src/main/resources/git.properties` and the info is
available in the object
`edu.berkeley.cs.amplab.adam.core.util.BuildInformation`.
- adam-cli: Add the command `buildinfo`.
@AmplabJenkins
Copy link

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/ADAM-prb/196/

fnothaft added a commit that referenced this pull request Mar 4, 2014
Display Git commit info on command line
@fnothaft fnothaft merged commit c5533e6 into bigdatagenomics:master Mar 4, 2014
@fnothaft
Copy link
Member

fnothaft commented Mar 4, 2014

Merged—thank you @smondet!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants