-
Notifications
You must be signed in to change notification settings - Fork 198
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
#69 fixed #71
#69 fixed #71
Conversation
private CommitUser createCommitUser() { | ||
CommitUser user = new CommitUser(); | ||
user.setName("site-maven-plugin"); | ||
user.setEmail("support@github.com"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ so all commits are going to show up as a different user than the one running the commit... really not a good idea AFAIC
These values should be set to the ones configured for the repository in the local git config or fallback to the global git config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally yes, but at the moment the plugin doesn't work at all. Version 0.9 is broken and nobody can use it. Would be great if we can merge this temporary fix and release version 0.9.1. Then, we can think about making it fully configurable, etc. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been unable to find the egit api docs; but I don't see why it should bypass the local git configuration stored in .git/config or the global config in the users home directory
I have several user accounts that I work with for different clients (so even using the {user.name}
provided by maven is not an option); this doesn't fix the problem for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So #72 should be merged instead of the other PR. Am I right ? |
@cescoffier fine for me, #72 should also work. please merge it and deploy a new version to Maven Central |
+1 for #72 |
Works for me, deploys the site without any issues