Skip to content

Add extended tone demo to examples #16

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

Merged
merged 3 commits into from
Aug 9, 2021

Conversation

apendley
Copy link
Contributor

Changes:

  • Added another example for playing tones using the MacroPad library.

Notes:

  • Please let me know if there's anything I need to change (e.g. the header, description, etc.), and I'll be happy to make the change!

@ladyada ladyada requested a review from kattni July 30, 2021 17:18
Copy link
Contributor

@kattni kattni left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

@kattni
Copy link
Contributor

kattni commented Aug 5, 2021

Hello, @apendley. As you can see the checks have failed. Please check out this guide and run Black on your code, and push the update to this PR. Thanks!

@apendley
Copy link
Contributor Author

apendley commented Aug 5, 2021

Understood! I'll take care of it as soon as possible. Thanks!

@apendley apendley requested a review from kattni August 6, 2021 04:08
@kattni
Copy link
Contributor

kattni commented Aug 6, 2021

@apendley It appears Pylint also failed with a trailing whitespace issue. This refers to whitespace at the end of a line or spaces on blank lines. I thought Black usually resolved that, so I'm a little confused. Are you up for troubleshooting it and trying to find the issue? If you feel like it's outside your wheelhouse, I can pull the PR down and then push the fix, but I wanted to give you the opportunity.

If you'd like to give it a try, you can install pre-commit, which is what we use to deal with all of this, and if you run it locally within the repository, you'll have the same processes happening on your computer that run remotely here. You can find instructions here on setting up Pre-commit.

Please feel free to ask any questions you might have about the process. I will be around again on Monday.

@apendley
Copy link
Contributor Author

apendley commented Aug 6, 2021

I don't mind giving it a shot! I would like to contribute more in the future, so it would be good for me to understand the process. I will try to fix this over the weekend if I get some time. Thanks!

@kattni
Copy link
Contributor

kattni commented Aug 6, 2021

Sounds great! Getting pre-commit setup will definitely help you in the future - we use it on all the libraries. The important thing to remember is that you install it globally once, but then you have to "install" it in each individual local repository by running pre-commit install from the top level of the repo directory. So it's a little bit of an extra step to remember to run that in each repo that you clone, but that's the way it's done. I'm happy to help with anything you need next week!

@apendley
Copy link
Contributor Author

apendley commented Aug 8, 2021

I installed pre-commit and fixed the issues it raised. Hopefully it will pass this time! 🤞

Copy link
Contributor

@kattni kattni left a comment

Choose a reason for hiding this comment

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

Thanks for sticking with this! Great job!

@kattni kattni merged commit 8a496b4 into adafruit:main Aug 9, 2021
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Aug 11, 2021
Updating https://github.com/adafruit/Adafruit_CircuitPython_MacroPad to 1.3.3 from 1.3.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_MacroPad#16 from apendley/extended_tone_demo

Updating https://github.com/adafruit/Adafruit_CircuitPython_MIDI to 1.4.2 from 1.4.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_MIDI#40 from jepler/no-reuse-exception
  > Merge pull request adafruit/Adafruit_CircuitPython_MIDI#39 from jepler/use-generator
  > Merge pull request adafruit/Adafruit_CircuitPython_MIDI#38 from EMATech/mtc

Updating https://github.com/adafruit/Adafruit_CircuitPython_MiniMQTT to 5.1.1 from 5.1.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_MiniMQTT#87 from DynamicDevices/ajl/fix-multiple-pingreqs

Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA:
  > Added the following libraries: Adafruit_CircuitPython_Ducky
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