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

NanoPi Duo pin mappings added #20

Merged
merged 9 commits into from
Jan 1, 2018
Merged

NanoPi Duo pin mappings added #20

merged 9 commits into from
Jan 1, 2018

Conversation

sgjava
Copy link
Collaborator

@sgjava sgjava commented Dec 31, 2017

Feature we discussed in #16

@rm-hull
Copy link
Owner

rm-hull commented Dec 31, 2017

The QA build failed - see https://travis-ci.org/rm-hull/OPi.GPIO/jobs/323631901#L563

You should pip install flake8 locally and fix the warnings that flake8 is complaining about.

@sgjava
Copy link
Collaborator Author

sgjava commented Dec 31, 2017 via email

@sgjava
Copy link
Collaborator Author

sgjava commented Jan 1, 2018

Looks like the failure in the test is not caused by the code I added. Can you please verify?

@rm-hull
Copy link
Owner

rm-hull commented Jan 1, 2018

No, not caused by your changes. There are a couple of intermittent failures ... See #19

Copy link
Owner

@rm-hull rm-hull left a comment

Choose a reason for hiding this comment

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

👍 All good, just a few tiny observations.
Also, add your name & github handle to CONTRIBUTING.rst as well

nanopi/duo.py Outdated
@@ -0,0 +1,49 @@
# -*- coding: utf-8 -*-
# Copyright (c) 2017 Richard Hull
Copy link
Owner

Choose a reason for hiding this comment

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

Please change to: # Copyright (c) 2018 Richard Hull & Contributors

nanopi/duo.py Outdated
# See LICENSE.md for details.

'''
Created on Dec 31, 2017
Copy link
Owner

Choose a reason for hiding this comment

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

No need to include created date & author - this can be obtained from git log, but worthwhile setting docstring header to something like:

Alternative pin mappings for NanoPi DUO board (see http://...)

nanopi/duo.py Outdated

Usage:

import nanopi.duo
Copy link
Owner

Choose a reason for hiding this comment

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

Use the RST code formatting extensions, so this would become:

Usage:

.. code:: python
   import nanopi.duo

   GPIO.setmode(nanopi.duo.BOARD)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK, bringing this up to standards :)

rm-hull
rm-hull previously approved these changes Jan 1, 2018
Copy link
Owner

@rm-hull rm-hull left a comment

Choose a reason for hiding this comment

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

👍 - good stuff, will roll out 0.3.1 shortly with this in

@sgjava
Copy link
Collaborator Author

sgjava commented Jan 1, 2018

Cool, it will be nice not to use my fork :)

rm-hull
rm-hull previously approved these changes Jan 1, 2018
@rm-hull rm-hull merged commit 173d15a into rm-hull:master Jan 1, 2018
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.

2 participants