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

Coding Guidelines #238

Closed
heynemann opened this issue May 16, 2013 · 13 comments
Closed

Coding Guidelines #238

heynemann opened this issue May 16, 2013 · 13 comments

Comments

@heynemann
Copy link
Contributor

As asked by @jdavid I'd like to start a thread here on the coding guidelines adopted by pygit2 as I got a little confused when submitting my pull request.

Thanks a lot for the help in getting my code merged.

@cholin
Copy link

cholin commented May 18, 2013

I wrote something about coding guidelines in the wiki. See Guidelines and Conding-Conventions. These are some proposals which @jdavid, @esc and I discussed at git-merge.

@heynemann
Copy link
Contributor Author

I edited the wiki a little bit to make it a little clearer. I think the Guidelines text needs some improvement.

In the High Level API bit, I didn't quite get the last two bullets. If there's any way to say it in a clearer way I think it'd be very helpful for newcomers.

@cholin
Copy link

cholin commented May 20, 2013

I refactored the text a litle bit. Hopefully it's clearer now.

@heynemann
Copy link
Contributor Author

It does help! Thanks!

@jdavid
Copy link
Member

jdavid commented May 20, 2013

A quick note. For the C coding style, we could simplify by just saying we follow PEP7

Regarding the Python coding style I would like to make some exceptions to PEP8, I will comment on it later. (By the way, if you run pep8 on the standard library you get tons of errors.)

@heynemann
Copy link
Contributor Author

True, but the standard library is not a great example in Python 2.7 (in
python 3.3 it's A LOT better).

Just look at modules like Queue (a module with camel-cased naming).

Cheers,

Bernardo Heynemann
Developer @ globo.com

On Mon, May 20, 2013 at 11:35 AM, J. David Ibáñez
notifications@github.comwrote:

A quick note. For the C coding style, we could simplify by just saying we
follow PEP7 http://www.python.org/dev/peps/pep-0007/

Regarding the Python coding style I would like to make some exceptions to
PEP8, I will comment on it later. (By the way, if you run pep8 on the
standard library you get tons of errors.)


Reply to this email directly or view it on GitHubhttps://github.com//issues/238#issuecomment-18150608
.

@cholin cholin mentioned this issue May 22, 2013
@jdavid
Copy link
Member

jdavid commented May 26, 2013

Regarding coding style now pep8 does not produce any error message.

There are few things I do not like about PEP8, but the one that bothers me most is to use only one blank line to separate methods. I find two blanks lines to be more readable. So I have configured pep8 to ignore this PEP8 rule, and updated the wiki accordingly.

If somebody disagrees just speak.

@cholin
Copy link

cholin commented May 27, 2013

I added a "valgrind memleak check"-note to the Guidelines.

@cholin
Copy link

cholin commented May 28, 2013

I think we should refactor the pygit2 code base a bit. There are tons of warning messages for pygit2 in valgrind relating to uninitialized variables. See the log this: valgrind.log. Additionally there is a verbose output for pygit2 with cpychecker (mainly missing error handling): cpychecker.log

@jdavid
Copy link
Member

jdavid commented May 28, 2013

@cholin do you fix it 😃 ?

@tmr232
Copy link
Contributor

tmr232 commented Apr 6, 2017

Can this issue be closed? It is very confusing when trying to understand if guidelines are set or not.

@esc
Copy link
Contributor

esc commented Apr 6, 2017

Probably.

@tmr232
Copy link
Contributor

tmr232 commented Apr 6, 2017

@jdavid , can you close the issue?

@jdavid jdavid closed this as completed Apr 6, 2017
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

No branches or pull requests

5 participants