search

Protonerd / FX-SaberOS

System code for Arduino based Lightsaber replicas for DIYino family boards
Creative Commons Zero v1.0 Universal
91 stars 42 forks source link

Fix invalid preprocessor directives #83

Closed per1234 closed 5 years ago

per1234 commented 5 years ago

There is no "#else if" preprocessor directive. The correct directive is #elif. "#else if" is treated as #else.

I recommend turning on compiler warnings (File > Preferences > Compiler warnings > All) in order to get helpful notifications of these sorts of issues.

jbkuma commented 5 years ago

I've tried testing this code and it doesn't not work correctly. The saber ends up in boot loops. I'm sure some, most, or even all of the changes may be valid, but some where they change the way something works and breaks it.

per1234 commented 5 years ago

The changes are all valid. They just exposed a bug in your code. The solution is to fix the bug, not to reject necessary changes.

jbkuma commented 5 years ago

While there may be improperly written code, a non-working pull request doesn't correct that. The code works as expected as it is currently written. The way to correct it is to spend time going through the code and testing that the right changes are made, then test it to ensure it still works as expected. We don't currently have the time to do that. We can approve a pull request only if it is tested and functional. Thank you for your time and effort, but if you aren't actually using and testing the code then an issue is probably a better course of action than an invalid pull request.

On Sun, Feb 17, 2019, 4:59 AM per1234 notifications@github.com wrote:

The changes are all valid. They just exposed a bug in your code. The solution is to fix the bug, not to reject necessary changes.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/Protonerd/FX-SaberOS/pull/83#issuecomment-464436713, or mute the thread https://github.com/notifications/unsubscribe-auth/ATdCMA8JB_-G_7SQPVfjCmcAE1qnTw5jks5vOSf5gaJpZM4Zlvk3 .