-
Notifications
You must be signed in to change notification settings - Fork 834
Library Maintainers Guide
All of the code in this library (including examples and tools) tries to comply with various style guides. We also have a automated zero-tolerance for lint errors. All commits are subjected to automated style guide & lint checks as part of the checks. You should run these checks prior to committing or sending a PR etc.
- Spaces, not tabs. Indents with two spaces.
- Limit line lengths to 80 characters. URLs are exempt.
- For overall style consistency, if there is an available ''Google'' code style guide, follow that.
- Use meaningful identifiers. Single letter variables etc are really only acceptable for iterators on small loops.
- Write your code so that it can be unit tested.
- Write unit tests.
- These are a guide only, not rules. They can be broken when appropriate. The universe will not end. ;-)
- Style guide: Google C++ Style Guide
- Linter: cpplint
- Checks enforced for: *.{h,c,cc,cpp,hpp,ino}
- All internal library code must use c99 exact-width type definitions.
e.g.
uint16_t
instead ofunsigned int
etc.- Avoid using
auto
in the library's core code as this can cause bit-wise issues that won't be detected in the Unit Tests. Use ofauto
in the unit tests is fine.
- Avoid using
- Style guide: Google Python Style Guide
- Linter: pylint with pylintrc config.
- Automatic code style formatting: yapf with a .style.yapf config. e.g. Google and two-space indents.
- Checks enforced for: *.py
- Please write unit tests for your code. They are preferred, not a requirement. However it is good engineering practice to write them.
- Indicate in your commits/PR if you have/haven't actually tested the code, especially if there are no Unit Tests for it.
- Run all the existing unit tests before you commit/create a PR. GitHub Actions will also do it but it will email everyone saying something broke. It will save you some embarrassment. ;-)
- It is "best practice" to have your code reviewed by someone else prior to submission. Do it, but it's not a hard requirement.
- If no changes have been requested by a reviewer after a week or so (including no response) you can submit the code.
- You only need a single review approval before merging.
- As a reviewer, try to offer constructive advice and if it is trivial, make the change required in the PR if you can.
- Be kind to first-time or novice contributors, but don't compromise the library to accommodate them obviously.
- Emergency non-reviewed commits & PRs are fine. Use your common sense. i.e. A trivial change that fixes a bug. Typos etc.
TL;DR: "What the job of a code reviewer entails is someone to check if the code looks sane, would be similar to how they would implement it, check for any obvious boo-boos, offer advice, and offer constructive criticism. A code review(er) is not responsible for making sure there are no bugs, or anything. They are never to blame if something goes wrong. All responsibility is with the code author basically. Not you. So there is little pressure." Quote ref
The Unit Tests which are under the test/ & tools/ directories are for a Unix machine, not the micro-controller (ESP8266). e.g. For GitHub Actions & the developer's workstation.
All internal library code must use c99 exact-width type definitions.
e.g. uint16_t etc.
You must disable any Arduino/ESP8266 specific code (e.g. Serial.print()
etc.) using something like:
#ifndef UNIT_TEST
<Arduino specific code ...>
#endif
The Unit Tests & Test Coverage are not perfect as we can not emulate hardware specific features and differences. e.g. Interrupts, GPIOs, CPU instruction timing etc, etc.
The example code has no unit tests.
To run the tests yourself, try the following:
$ cd test
$ make install-googletest # Only ever needs to be run/installed once.
$ make run_tests
This library uses GitHub Actions to ensure all the tests & code style checks pass before allowing a commit/merge to the library. i.e. You might as well run them by hand before you commit your final code. There is no escaping these checks. The full battery of checks performed can be found in the library's .github/workflows/*.yml files. However, the Lazy Developer's guide is as follows.
- Install the appropriate tools. This only needs to be done once. e.g.
$ # This all assumes you've got a linux machine with typical dev tools installed. e.g. gcc, g++, python, make, etc.
$ sudo apt-get install pylint3 # Install the Python Linter
$ sudo pip install platformio # PlatformIO is like the Arduino IDE. It will build/compile the code.
$ pio update # Ensure PlatformIO is to date.
$ wget https://raw.githubusercontent.com/google/styleguide/gh-pages/cpplint/cpplint.py # Get the latest C++ linter.
$ cd git/IRremoteESP8266/test
$ make install-googletest
Did you change a file under the examples directory?
- Ensure that the example code still compiles without any errors or warnings.
- Run the C++ linter from the top directory of the library. Fix any issues it finds. e.g.
$ cd git/IRremoteESP8266
$ cpplint --extensions=c,cc,cpp,ino,h --headers=h,hpp --count=total --verbose=2 {src,src/locale,test,tools}/*.{h,c,cc,cpp,hpp,ino} examples/*/*.{h,c,cc,cpp,hpp,ino}
or
$ cd git/IRremoteESP8266
$ wget https://raw.githubusercontent.com/google/styleguide/gh-pages/cpplint/cpplint.py
$ python ./cpplint.py --extensions=c,cc,cpp,ino --headers=h,hpp {src,src/locale,test,tools}/*.{h,c,cc,cpp,hpp,ino} examples/*/*.{h,c,cc,cpp,hpp,ino}
Did you change a file ending in *.h
or *.cpp
that was not under the examples directory?
- Run the C++ linter from the top directory of the library. Fix any issues it finds. (See above for examples)
- Run all the Unit Tests in the test directory. e.g.
$ cd test
$ make install-googletest # Only ever needs to be run/installed once.
$ make run_tests
Did you change anything under the tools directory?
- Make sure the program you edited still compiles/works as expected.
- Run the C++ linter (See above)
- Run the "tools" tests. i.e.
$ cd tools
$ make run_tests
- You need to run the python linter. e.g.
$ sudo apt-get install pylint3 # Note: Only needs to be installed/run once.
$ cd git/IRremoteESP8266
$ pylint3 -d F0001 {src,test,tools}/*.py
$ cd test
$ make run_tests
$ cd ../tools
$ make run_tests
To reduce useless noise in the commit log, please use Squash and Merge option to reduce your PR into a single commit before it is merged into the main code branch.
- Make sure your master branch is up-to-date.
- Warning: The following will cause any local changes to be lost.
git fetch origin git checkout master git reset --hard origin/master git clean -f -d
-
git checkout -b vXX.YY.ZZ
Where vXX.YY.ZZ is the new version string. e.g. v2.3.2 - Update/Re-generate the Doxygen API documentation pages.
- Install doxygen if you haven't already.
- In the root of the project, run the commands:
rm -rf docs/doxygen/html doxygen
- If there is no output from the command, all went well! Huzzah.
- If there are errors or warnings, fix/address them (in the normal PR manor) before proceeding.
- Perform the following commands:
git add docs/doxygen/html git commit -m "Regenerate Doxygen documentation for new release"
- Change the library version in the code:
- Update
_IRREMOTEESP8266_VERSION_(MAJOR|MINOR|PATCH)
insrc/IRremoteESP8266.h
to the new version number. - Update
version
inlibrary.json
&version=
inlibrary.properties
accordingly. - All three must be the same.
- Update
- Update the version information in the documentation:
- Update "Now Available" sections in
README.md
&README_??.md
. - Update
ReleaseNotes.md
and add a new section for the new release.-
git log
should show you the change history. - Update the date to today's date for the new release.
-
- Update "Now Available" sections in
- Run
tools/mkkeywords > keywords.txt
from the top directory. (Requires a Linux machine etc) - Run
tools/scrape_supported_devices.py
from the top directory. (Requires Python3) -
git diff
and review the changes are what you expect. -
git commit -a
and create an appropriate entry. e.g. "v2.3.2 release" git push --set-upstream origin vXX.YY.ZZ
- Wait for the GitHub Actions to complete successfully.
- Create a Pull Request to the 'master' branch:
- Set appropriate reviewers as needed.
- Copy in the new section of the
ReleaseNotes.md
as the PR comment.
-
Wait for:
- Approval (or not, all the substantial commits should have already been done)
- GitHub Actions to pass
- Only then, merge to master.
- Go to the Releases tab in github.
- Click "Draft a new release".
-
Tag Version
: vXX.YY.ZZ -
Target
: Master -
Release title
: IRremoteESP8266 vXX.YY.ZZ -
Notes
: Cut & paste from theReleaseNotes.md
update. - Make sure the pre-release checkbox is un-checked.
- Click the Preview tab and check it looks okay.
- Publish Release
- The new release is now live. Arduino IDE's Library Manager should pick up the new version within 24 hours.
- Publish the documentation (e.g. API docs) to Github Pages.
- Perform the following commands:
git fetch git checkout gh-pages git reset --hard origin/gh-pages git clean -f -d git merge -s subtree origin/master git push
- The documentation should now be live at: https://crankyoldgit.github.io/IRremoteESP8266/
- API documentation: https://crankyoldgit.github.io/IRremoteESP8266/doxygen/html/
- Perform the following commands:
- Update any reported bugs that were fixed by changes that happened between this new version and the now previous one.
- Relax. Grab a cold beverage. It's all done.