bxparks / AceButton

An adjustable, compact, event-driven button library for Arduino that debounces and dispatches events to a user-defined event handler.
MIT License
393 stars 37 forks source link

Use arduino_ci for testing #63

Closed ianfixes closed 3 years ago

ianfixes commented 3 years ago

Full disclosure: I'm the owner of the arduino_ci project, and this is not meant as an attempt to convince you to use it.

This was my own exercise to see what shortcomings might exist in my software (I found a few) and I'm sharing the results here. Until now I had been testing against libraries that did not already have a unit testing capability, which meant that I had no idea whether I was covering all of the features one might expect. AceButton has some well-developed testing processes, and so it was satisfying to me to see the passing build status on my branch (hopefully indicating that I've achieved some level of parity with AUnit).

Since you are also the maintainer of a build system for Arduino libraries, I would welcome your feedback (as candid as possible, positive or not) on these changes and the build logs themselves. Does it look sensible? Does it convey the information you want? Am I passing things that should fail? etc.

Summary of changes:

bxparks commented 3 years ago

Hi, Just to make sure that I understand the purpose of this PR, you are not actually requesting this to be merged into the AceButton code, right? Because I don't think I would ever merge this. So the purpose of this is to discuss the features and potential deficiencies of your arduino_ci framework. I would be willing to engage in some of that, but I don't think this PR is the right place for that discussion. Doing that here would generate spam for the subscribers of this project, most of which would be irrelevant to them.

Since you have forked AceButton, I think a better way would be for you to create a PR of this change into your fork, then add me as a reviewer of that PR, so that we take this discussion off of this project. Does that sound reasonable?

ianfixes commented 3 years ago

I don't think I would ever merge this

The reasoning behind this is precisely what I'm interested in learning. I hope we can focus on this in the PR discussion in my own branch https://github.com/ianfixes/AceButton/pull/1