salarkhan / git-pear

swap credentials after each commit
3 stars 0 forks source link

To quote salar: #28

Open supertopher opened 10 years ago

supertopher commented 10 years ago

"man, I'm super proud of what we've done here.

I would welcome all derogatory comments, since all they'll do is make this project sicker "

have at it plx @quackingduck @mweitzel

please note this is intended for a brew install so at the moment for testing as written the path must be amended to

PATH="$PATH:."

pear calls the command pear for functionality. If it's not in the path life is unhappy

it also probably needs an API key as i assume we will run into the same rate limiting problem we currently experience with iam weare

i don't think me or salar are opposed to some DBC branding in the read me or usage for shits and giggles. Wherever it goes i had fun writing this.

salarkhan commented 10 years ago

it also probably needs an API key as i assume we will run into the same rate limiting problem we currently experience with iam weare

Actually, this is only the case if we want users to be able to use their usernames instead of emails. ( Which we do. I think )

As it stands, we never hit an API. Therefore, no rate limiting problem!

quackingduck commented 10 years ago

/cc @mattbaker

This is really interesting. I have a number of comments but I'll try not to overload you. Two initial things:

  1. You've got to provide people with an easy way to install this without saying "it's intended for homebrew". Look at the install script in shazzam which creates a symlink in /usr/local/bin that's good enough.
  2. The readme should show an example git session where this thing is working, something like:
$ cd /tmp
$ mkdir repo
$ cd repo
$ git init
$ git init
Initialized empty Git repository in /private/tmp/repo/.git/
$ git commit --allow-empty -m .
[master (root-commit) 66d54e2] .
$ git commit --allow-empty -m .
[master 9b75c03] .
$ git commit --allow-empty -m .
[master 5baa9ba] .
$ git commit --allow-empty -m .
[master ad1bb09] .
$ git log
commit ad1bb09f85acc591816f3c93eb2140128cfedec3
Author: Myles Byrne <myles@myles.id.au>
Date:   Tue Jul 8 17:40:36 2014 -0700

    .

commit 5baa9ba8a3046e2e1e62eab413072dba43cc0c6f
Author: Myles Byrne <myles@myles.id.au>
Date:   Tue Jul 8 17:40:35 2014 -0700

    .

commit 9b75c031d8dec89b4f25923ec6bba5c57640f04f
Author: Myles Byrne <myles@myles.id.au>
Date:   Tue Jul 8 17:40:34 2014 -0700

    .

commit 66d54e2ded8637a9519cf0297f03da8b69dc72ca
Author: Myles Byrne <myles@myles.id.au>
Date:   Tue Jul 8 17:40:30 2014 -0700

    .

... except obviously the author and committer values should be alternating.

mattbaker commented 10 years ago

:thumbsup: to Myles' feedback.

What kind of feedback are you guys looking for? Technical? About the idea itself?

salarkhan commented 10 years ago

@quackingduck Hook it up with access rights to shazzam & we'll make it happen @mattbaker Both? Anything? Everything? Haha I feel like this dog when I'm writing bash, soooo I guess technical/architecture feedback would be a sweet place to start. Especially since Myles has already given some solid actionable feedback on the UX

supertopher commented 10 years ago

@mattbaker i'm pretty happy to hear that any part sucks so we can make it better. Whatever you see that needs improvement please don't hesitate. Also feel free to tell us the concept sucks, i may disagree but by all means feedback++

supertopher commented 10 years ago

@salarkhan you now have access to shazzam

salarkhan commented 10 years ago

@quackingduck @mattbaker : Changes have been made. Ready for round 2.

I vaguely remember you saying something about using git config instead of a .txt file?

quackingduck commented 10 years ago

@salarkhan use git log --format=fuller to show that the program cycles the author and the commiter

quackingduck commented 10 years ago

And let's talk about add:

You can stuff directly to the .git/config file with:

$ git config --add pear.email alex@example.com
$ git config --add pear.email bob@example.com
$ git config --get-all pear.email
alex@example.com
bob@example.com

The only real advantage here is that you don't have to parse your custom text file.

Also, treat the list of emails (the "team") as a set: if someone is trying to add an existing user to the set then just do nothing. Don't print a message. And also, when you do print messages, print them to stderr (which should really be called "standard message") and not stdout which is for parsable program output

quackingduck commented 10 years ago

Also, I'm too dumb to work out the logic here: https://github.com/salarkhan/git-pear/blob/a25cd4cbd48e6d07e6c184bd77676cd7278abcc5/libexec/set-author-committer#L8

Maybe this project could also include in implementation.md which wouldn't explain how to use it but would explain how it works.

salarkhan commented 10 years ago

I liked the idea of using git config until I ran into a problem when cycling the team.

Current implementation of cycle is: Remove the top line, append it to the bottom. Problem is, git config --unset fails if there is more than 1 value to a key. AKA I can't remove the "top" value.

A way around this I figured was:

Kinda hairy.

I have a wayyyyyyyyyy cleaner implementation if we only ever have to worry about 2 users. However, I'm unsure whether it's cool that git-pear can't support trios.

( I think the question I'm asking here is, do trios occur enough to warrant the extra code? )

salarkhan commented 10 years ago

As to the rest of your comments, I fully support an implementation.md, since figuring out how to get the post-commit hook to play nicely was a pain.

Also, rule of silence. Got it.