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

contributing docs updates #120

Merged
merged 4 commits into from
Jun 17, 2019
Merged

contributing docs updates #120

merged 4 commits into from
Jun 17, 2019

Conversation

Islast
Copy link
Collaborator

@Islast Islast commented Jun 13, 2019

  • I'm ready to merge
  • What's the context for this pull request?
    (this is a good place to reference any issues that this PR addresses)
    Issues Update Development guide: Installation instructions #110 and import inside functions #117

  • What's new?
    updated install instructions as suggested in Update Development guide: Installation instructions #110
    following import inside functions #117 I have added a short section to the development guide in which to include clarifications on style.

  • What should a reviewer feedback on?
    @KirstieJane I like to keep you pinged in any changes to the contributing docs. It would be great if you could take a glance over and make sure they read well.
    @wingedRuslan I've added some bullet points to the development guide. I'd like to know: would seeing something like this have eased the concerns you raised in import inside functions #117? Or was your question a more general one about wanting to know when it was appropriate to change existing code to fit PEP8 style? I'm trying to feed this question back into the developer's guide it would be great to hear your opinion

  • Does anything need to be updated after merge?
    (e.g the wiki or the WhitakerLab website)
    nope :)

@wingedRuslan
Copy link
Collaborator

@Islast actually I was wanting to know if it was appropriate to change existing code to fit PEP8 style. So it was more a general question. From my perspective, it is unnecessary information to include in the development guide. We will refactor the code(imports +shebangs) so that it won't be confusing to new contributors.

@Islast
Copy link
Collaborator Author

Islast commented Jun 14, 2019

Thanks for the feedback Ruslan. I've changed it to simply note that contributors should feel free to update existing code when it breaks PEP8

Copy link
Member

@KirstieJane KirstieJane left a comment

Choose a reason for hiding this comment

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

GREAT! Thank you!


### Linting

scona uses the [PEP8 style guide](https://www.python.org/dev/peps/pep-0008/).
You can use [flake8](http://flake8.pycqa.org/en/latest/) to lint code.
We're quite a young project (at time of writing in January 2019) and so we aren't going to be super hardcore about your linting!
Linting should make your life easier, but if you're not sure how to get started, or if this is a barrier to you contributing to `scona` then don't worry about it or [get in touch](CONTRIBUTING.md#how-to-get-in-touch) and we'll be happy to help you.
Feel free also to correct unlinted code in scona when you come across it!:sparkles:
Copy link
Member

Choose a reason for hiding this comment

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

AMEN! 🙌

@KirstieJane
Copy link
Member

Ready to merge @Islast 🚀

@Islast Islast merged commit 1e70f46 into WhitakerLab:master Jun 17, 2019
@Islast Islast mentioned this pull request Sep 20, 2019
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