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 log_append to reference. #277

Merged
merged 1 commit into from
Nov 13, 2013
Merged

Add log_append to reference. #277

merged 1 commit into from
Nov 13, 2013

Conversation

xtao
Copy link
Contributor

@xtao xtao commented Oct 30, 2013

Add log_append to reference.

@jdavid
Copy link
Member

jdavid commented Oct 31, 2013

First thanks for your contributions.

Things to change:

  • Rename append_log to log_append
  • Make all parameters positional (so mandatory). If the user does not want a message, then she must pass None.

Question, why you don't take the oid as parameter? do you have a reason?

Also, return None instead of True (it only makes sense to return True if somewhere else you return False, which is not the case).

@xtao
Copy link
Contributor Author

xtao commented Nov 1, 2013

@jdavid I fixed:

  1. rename function name
  2. optional parameter message.
  3. return None

But, use oid as parameter, do you mean take commit as parameter?

@jdavid
Copy link
Member

jdavid commented Nov 1, 2013

Yes, like:

ref.log_append(oid, signature, message)

And use /* */ style comments, as PEP7 states, http://www.python.org/dev/peps/pep-0007/

@alexband
Copy link

alexband commented Nov 1, 2013

PEP7 🍺

@tclh123
Copy link

tclh123 commented Nov 1, 2013

🍻

@jdavid
Copy link
Member

jdavid commented Nov 2, 2013

Could you please add unit tests?

@xtao
Copy link
Contributor Author

xtao commented Nov 3, 2013

@jdavid I'll add later :)

@jdavid jdavid merged commit e345d23 into libgit2:master Nov 13, 2013
@jdavid
Copy link
Member

jdavid commented Nov 13, 2013

Thanks @xtao for contributing!

Note I changed the prototype to make it more consistent with rest of the API.

@xtao xtao deleted the reflog branch January 29, 2014 06:49
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.

None yet

4 participants