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

DOC: Refer to PyTroll coding guidelines #659

Merged
merged 1 commit into from Mar 18, 2019

Conversation

gerritholl
Copy link
Collaborator

In the documentation Developer Guide, refer to the general PyTroll coding guidelines. This replaces the much less complete paragraph for SatPy coding guidelines.

In the documentation Developer Guide, refer to the general PyTroll
coding guidelines.
@djhoese
Copy link
Member

djhoese commented Mar 14, 2019

I am ok merging this, but only after the linked to guidelines are updated to reflect current practices:

  1. 120 max line length (I think we've completely gotten rid of 80 as a limit)
  2. The "python3" compatibility could probably just be removed at this point since most/all projects will drop python 2 support by the end of 2019.
  3. I think @mraspaud's maintainability guidelines are too strict for the average developer. IMO they end up making the code needlessly complex/long when they're supposed to do the opposite. Perhaps emphasizing limiting the number of the things mentioned (lines of code, parameters, etc) would be less scary? Or maybe start the section out with nicer language. I understand guidelines are guidelines, but still.
  4. Mention "flake8" as a tool for checking style in style checking. Could also mention stickler which is used in most repositories PRs.
  5. Remove link to releasing instructions and/or mention "RELEASING.md" instructions that should be in most if not all repositories.

@codecov
Copy link

codecov bot commented Mar 14, 2019

Codecov Report

Merging #659 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #659   +/-   ##
=======================================
  Coverage   78.53%   78.53%           
=======================================
  Files         138      138           
  Lines       20073    20073           
=======================================
  Hits        15765    15765           
  Misses       4308     4308

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8eea7cf...a5e21ad. Read the comment docs.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.005%) to 78.538% when pulling a5e21ad on gerritholl:dev-guide-reference into 8eea7cf on pytroll:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants