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

Write contribution guidelines #101

Closed
drvinceknight opened this issue Mar 8, 2015 · 12 comments
Closed

Write contribution guidelines #101

drvinceknight opened this issue Mar 8, 2015 · 12 comments

Comments

@drvinceknight
Copy link
Member

I'd suggest:

  • Follow PEP guidelines
  • No use of external library unless really necessary (matplotlib for example is deemed necessary but has exception handling so that code can be run without it).
  • 100% test coverage (a very nice test suite has been written that automates a lot of the testing)
@drvinceknight
Copy link
Member Author

This should wait till #100 and #99 are taken care of. See #98.

@meatballs
Copy link
Member

and meaningful variable names (please)!!

@drvinceknight
Copy link
Member Author

Yup, great will add that also.

I'm thinking of making pep8 be part of the testing suite on Travis... Any reason not to? (330 things fail pep8 at the moment)

@langner
Copy link
Member

langner commented Mar 9, 2015

I hate to play the gadly again, but there is no real reason for this and it would be a distraction. Do you really need code that is squeaky clean? Style guides are just that, guides. Not rules. See this part of PEP8 itself:
https://www.python.org/dev/peps/pep-0008/#a-foolish-consistency-is-the-hobgoblin-of-little-minds

@meatballs
Copy link
Member

I tend to agree with @langner

Style guidelines are nice to have, but can end up causing more problems than they solve.

For example, some of the PEP8 errors I've fixed along the way have been simply that a line is more than 79 characters. It simply isn't worth the effort to go through the codebase to find and correct them.

@meatballs
Copy link
Member

To be clear - I think the contribution guidelines should include PEP, but there's no point including compliance as part of the test suite.

@drvinceknight
Copy link
Member Author

Cool, that sounds good to me, will leave the PEP testing out of Travis but
will include something about PEP in the guidelines. I'll throw them on a
separate branch (might get to it today) and ping you guys to see if you
think what I've written is good :)

On Mon, 9 Mar 2015 15:01 Owen Campbell notifications@github.com wrote:

I tend to agree with @langner https://github.com/langner

Style guidelines are nice to have, but can end up causing more problems
than they solve.

For example, some of the PEP8 errors I've fixed along the way have been
simply that a line is more than 79 characters. It simply isn't worth the
effort to go through the codebase to find and correct them.


Reply to this email directly or view it on GitHub
https://github.com/drvinceknight/Axelrod/issues/101#issuecomment-77869555
.

@drvinceknight
Copy link
Member Author

Hey @langner and @meatballs , I've just pushed this as contribution guidelines: https://github.com/drvinceknight/Axelrod/blob/c-guidelines/docs/contributing.rst

Let me know what you think. If I don't hear anything I'll assume it's ok (kind of a guidelines/tutorial ish thing).

Once that's merged in to master I'm going to try and go to down on documentation... (by the way: I'm writing this using wifi on a plane! I can't get over how cool that is... I'm in the future!)

@langner
Copy link
Member

langner commented Mar 14, 2015

I skimmed over it and it seems fine.

@drvinceknight
Copy link
Member Author

👍

@meatballs
Copy link
Member

Sorry - been down with a dose of food poisoning! Looks good to me too.

@drvinceknight
Copy link
Member Author

Cool, thanks both. That is now merged in to master.

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

3 participants