-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Fix spelling #998
Fix spelling #998
Conversation
.def(bp::init<uint16_t, uint16_t, uint32_t>((bp::arg("_cepin"), bp::arg("_cspin"), bp::arg("spispeed")))) | ||
.def(bp::init<uint32_t>((bp::arg("spispeed")))) | ||
.def(bp::init<uint16_t, uint16_t, uint32_t>((bp::arg("_cepin"), bp::arg("_cspin"), bp::arg("spi_speed")))) | ||
.def(bp::init<uint32_t>((bp::arg("spi_speed")))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only questionable change because it could break existing user code that invokes the optional SPI speed parameter as a keyword argument. The following patch would need to be applied if this is merged.
-radio = RF24(CE_PIN, CSN_PIN, spispeed=4000000)
+radio = RF24(CE_PIN, CSN_PIN, spi_speed=4000000)
although any code that invokes it as a positional argument should still work fine
radio = RF24(CE_PIN, CSN_PIN, 4000000)
I'm betting that most users are doing the second usage (if at all changing the default SPI speed) because the C++ docs (including COMMON_ISSUES.md) suggest doing it this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC, boost.python does not handle keyword arguments gracefully. It may be safe to merge this if spispeed=<int>
never worked previously.
Memory usage change @ 096d5b9
Click for full report table
Click for full report CSV
|
possibly breaking API
@@ -0,0 +1,313 @@ | |||
version: "0.2" | |||
language: en | |||
words: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tip
Any intentionally misspelled words are specified in this yaml list.
The cSpell VSCode extension provides a way to auto-add any intentionally misspelled words to this list using only the mouse (no need to manually type it in here).
Notice this list is sorted alphabetically.
Damn, those are a pile of spelling mistakes! |
And there's over 280 words intentionally misspelled including proper nouns (like products, brands, people's name/username) and some acronyms. Most of the exceptions are from not using snake casing or camel casing, so cspell can't identify the word "pigpio" in the phrase "libpigpio" (both would have to be ignored misspellings). |
Add spell checking
Locally, I've been using this cspell VSCode extension (for a while), but I think it would be better if the remote adhered to a lesser quantity of spelling errors.
In the bigger picture, I think this would help those using google to translate our English docs into their native tongue. For day-to-day development, this should prevent the numerous copied-and-pasted spelling errors (in examples and driver code)
I added a CI job to check spelling (which also uses cSpell) in all source files including
Note
Spell checking is not applied to the
configure
script nor any Makefiles.