Skip to content

Conversation

vise890
Copy link

@vise890 vise890 commented Feb 19, 2021

Context

Adds support for getting the properties git.commit.time.author and git.commit.time.committer.

Contributor Checklist

  • Added relevant integration or unit tests to verify the changes
  • Update the Readme or any other documentation (including relevant Javadoc)
  • Ensured that tests pass locally: mvn clean package
  • Ensured that the code meets the current checkstyle coding style definition: mvn clean verify -Pcheckstyle -Dmaven.test.skip=true -B

@TheSnoozer
Copy link
Collaborator

Hello,
thank you for your contribution! I think this creates a json-naming conflict similar to the one outlined in #122. Could you perhaps name the properties differently?

Perhaps simply:
git.commit.time.commit ("--pretty=format:%ct") => git.commiter.time
git.commit.time.author ("--pretty=format:%at") => git.author.time

@vise890
Copy link
Author

vise890 commented Feb 19, 2021

ah i see, i've renamed to git.committer.time and git.author.time (to stay in line with git.user...)

|`git.commit.message.full` | Represents the raw body (unwrapped subject and body) of the commit message (`git log -1 --pretty=format:%B`) |
|`git.commit.message.short` | Represents the subject of the commit message - may *not* be suitable for filenames (`git log -1 --pretty=format:%s`) |
|`git.commit.time` | Represents the (formatted) time stamp when the commit has been performed. |
|`git.commit.time.commit` | Represents the (formatted) time stamp when the commit has been performed. |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you perhaps also rename the remaining instances (including the ones in the tests) ? :-)

Sorry to be picky here.

Copy link
Author

Choose a reason for hiding this comment

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

no worries what so ever 😄 ! I think i got them all now?

@TheSnoozer
Copy link
Collaborator

Thanks again for the contribution! Looks good now.

@TheSnoozer TheSnoozer merged commit 5169379 into git-commit-id:master Feb 22, 2021
@TheSnoozer TheSnoozer added this to the 4.0.4 milestone Feb 22, 2021
@vise890 vise890 deleted the comm-auth-times branch February 22, 2021 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants