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

Add author to commits #28

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

knut-forkalsrud
Copy link

Git distinguishes between author and committer. The configured
committer details in site/settings/addons/spock.yaml are fine,
but the Statamic user who is logged in should be credited with
the authorship. The log show both. For example with spock.yaml
like the example:

git_username: Spock
git_email: [email protected]

And a user (username "bob", first name "Bob", last name "Author", email "[email protected]").
The log will look something like:

git log --pretty=full
Author: Bob Author <[email protected]>
Commit: Spock <[email protected]>

    Entry deleted by bob

This is somewhat opposite to the desires expressed in issue #25, having the commit message itself include something like "[spock]" to distinguish it from typical developer commits. However there is no conflict between this and that. We can do both.

My apologies for breaking all the tests. I'm not really a PHP developer, and my feeble attempt at running the test is greeted with a definite error message:

php addons/Spock/tests/GitTest.php 

Fatal error: Class 'PHPUnit_Framework_TestCase' not found in /Users/knut/opensource/spock/addons/Spock/tests/GitTest.php on line 10

I may need a pointer to how to run the tests effectively.

Also part of the pull request is a change from looking at "affected paths", and instead commit all outstanding changes. There seems to be an issue with some changes (draft blog entries) not being accurately reflected in "affected paths". Maybe this aspect should have ben a different pull request altogether.

Finally I'm using the standard escapeshellarg to escape any potential malicious input to the shell command. I notice commit b4a89a4 is opposite in some ways, but I suspect that is just a workaround for a different culprit.

Copy link
Contributor

@rrelmy rrelmy left a comment

Choose a reason for hiding this comment

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

Hey knut

I really like the feature
I added some feedback 😉

addons/Spock/Git.php Outdated Show resolved Hide resolved
addons/Spock/Git.php Outdated Show resolved Hide resolved
*/
protected function author() {
$user = $this->user->toArray();
return sprintf("--author=%s <%s>", $user['name'], $user['email']);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use the getter methods on the User object?
Like $this->user->email()

Copy link
Author

Choose a reason for hiding this comment

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

Actually, I can't get $this->user->email() to work... Maybe I'm being dense, but PHP claim there is no such function. I'll go back to the toArray() approach.

* Create a Git author parameter from the user's name and email address,
* i.e, "--author=A U Thor <[email protected]>"
*/
protected function author() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This method could be private

Copy link
Author

Choose a reason for hiding this comment

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

Indeed it could be private, however all the other similar methods are protected, so I'd rather follow suit on that.

addons/Spock/Git.php Outdated Show resolved Hide resolved
Copy link
Author

@knut-forkalsrud knut-forkalsrud left a comment

Choose a reason for hiding this comment

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

Thank you @rrelmy for the review. I have tried to address the issues you raised. The only proposed changes are now in the construction of "git commit".

* Create a Git author parameter from the user's name and email address,
* i.e, "--author=A U Thor <[email protected]>"
*/
protected function author() {
Copy link
Author

Choose a reason for hiding this comment

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

Indeed it could be private, however all the other similar methods are protected, so I'd rather follow suit on that.

As I can't make $this->user->email() work.
@bluetidepro
Copy link

Would love to see this added! Is it also possible to have the commitMessage be something you can append or change via the config? Maybe something like this:

protected function commitMessage() {
    $msg = $this->label();
    if ($this->user) {
        $msg .= ' by ' . $this->user->username();
    }
    // Append commit if in config
    if ($commitAppend = array_get($this->config, 'git_commit_append')) {
        $msg .= ' ' . $commitAppend;
    }
    return $msg;
}

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.

3 participants