-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Adding more authorized contributors #525
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
Comments
Hi Rafi, The main effort must be put to clean up all the loose ends and tag the remaining issues and PR, to make it transparent at one look, what the state of the repo is. So I can offer to help maintaining this repo and try to give it a "fresh look" 😀 to keep its value for the Arduino IR community. |
Great! I've added you as a contributor. Please let me know if you are able to do things like close issue/change labels etc. |
Thanks.
|
And another question. I personally use mainly the master branch, if CI (by Github Actions or Travis) is installed. |
My responses below, apologies for the poor formatting! I wanted to get the info out quickly haha and thought it should be fine to just bullet point them.
Also, we do have TravisCI but testing is quite poor and I'm not even sure if the tests still work! I think we can use the dev branch similar to how you describe the master branch in your workflow? Also, in all of this just keep in mind this is how things are currently being done, and by no means does that mean this is how things should be done or has to be done. I strongly believe in developing software in the open and with community support and involvement. So if any of my choices don't make sense or are just simply bad choices then I invite further discussion and would love to hear alternatives. Even if you find something and are like "hmmm this seems like a bad idea" but don't have an immediate solution, then please still comment because at least we can identify issues :) Thanks for joining the project, It's going to be fun collaborating. |
Hi Rafi, According to adding new protocols, my opinion is, that this is a community driven thing, and times are changing, so why not include new protocols like #690 to avoid everyone to scan all the forks, if it is implemented in any of these. Regarding the master branch, I know this discussion since 30 years. There is no right or false, it is merely a matter of taste. Ps. Please keep in mind that I am from Germany, so my english may "sound" strange. |
I am a bit lost. |
I would welcome if this project starts moving again. Some thoughts:
More later!
Überhaupt kein Problem! |
Thanks a lot for this valuable link!!! I came from SVN and are still accustoming with Git... But one remark. All these concepts are targeting on teams working simultaneous on one project. and rely on everyone to know the rules and semantics of the branches. |
Loving this discussion! My thoughts below.
I would like to come up with a design that makes it easy to add these protocols. And also that the end user shouldn't have to do much work. Not sure yet what the design could be. Maybe Protocols become sketches instead? That users can copy around, but this may be too much work for them.
That's great to hear! One of the main reasons I couldn't support all the boards the community wanted was a lack of maintainers who could test. Now we don't have to worry about that.
Yes the branching so far has been a bit wonky. Likely we want to do a beta+dev merge and then merge master into dev and work from dev. More below.
Yes that actually makes a lot of sense. I guess my concerns are:
I guess I'm really trying to lower the barrier and remove any potential obstacles, such as users accidentally using dev code. I think from the development side it makes perfect sense for trunk-based and I'm starting to lean towards that a lot more on my private projects. However, in this case I am more for optimizing for the users and keeping ithem safe. Also, we do have a README but not very many people read it.. Its just a reality haha. That's one reason I'm inclined to keep master stable because of the many tutorials that ask users to download master. Looking forward to hearing thoughts! P.S Your English is great! No disclaimers needed haha |
OK, I think I begin to understand you. I develop a feature locally until it works and then make a few compile checks with Sloeber. My first goal is to support the plain Arduino user, which looks first in the Arduino Library Manager (our Releases) or second, downloads the master, where he expect the latest bug fixes. And for Issues I like to patch the master and tell to download and test it, before I build a new release (for Arduino). For Pull Request it maybe a benfit to use the dev branch so we should advise collaborators to target pull requests to the dev branch? And I hate stale branches like experimental or feature_xyz, where, after 2 years, nobody knows what is contained in there.
A possible solution for supporting multiple protocols is to set symbols for conditional compiling like
I try to keep attention to the readme by putting pictures in it, but it's just a guess. BTW, If we overhaul this library what's about the library 1.5 specification? It makes the readme more prominent! |
IIRC, the "beta" branch goes a long way here. And, while on it, use the mechanisms of the library specification to not export IRremoteInt.h, differently put prohibit user's sketches from
(There is a reason for the name, at least there should be...) |
Who merges the two branches dev and beta? |
I am starting to be convinced that we should use a trunk (master) and not a dev branch. If we can reach a consensus then let's do that.
If you can create a PR to merge beta into dev (with any necessary changes) then the three of us can go over it and merge it into dev. I think once we have two branches, ie. dev and master then we can revampt and clean dev. Once dev is in a good state then we can merge dev into master and from then on just do our work on master. My only concern with using only master is that the code doesn't get reviewed. This may just be my old-style thinking.. But ideally all changes to the library go through a second pair of eyes. Even if the change were to come from me, I'd sleep better knowing that someone else had looked it over before it went into master. On the one hand I want changes to go into master quickly and to be tested and landed to users. On the other hand I dont know if code that hasn't been reviewed should go into master. What if we do this.
I think this is what feature branches are?
Yep! And if beta has that then lets use it. Anything we can do to improve the library makes sense for us to do.
Agreed
I think that's an option, Maybe once we have done the initial cleanup of the library we can revisit this with a few designs and then choose and implement one. We can safely disregard "professional c" restrictions that don't apply to Arduino haha :) |
Also forgot to comment on this
I think you're spot on with this. We need to "ship" more frequently so that we get user testing |
Hi Rafi, I had a closer look at the current state of dev and beta and the conclusion is: Because beta is in a good state, I will check it the next days with extended CI checks for other platforms and test them on my boards. After this I will create the PR from beta to master. I would like to keep the version number 3.0.0 for the new cleanup and Include handling. |
Hi ArminJo, All of that sounds good to me! |
I think that is a good choice. IMHO, more important that what branch is that a choice is made, and that the workflow is transparent and defined.
Great! Feel free to ask me for reviews for individual PRs. Protocols: before the "dust has settled", I recommend not accepting new protocols. Hardware: I strongly recommend treating #437 with priority, and only then accepting new hardware ports on top of that. |
I started today with testing 5 examples. In order not to get confused I always add the line In other libraries I even use the line: Thats it for today. |
Awesome! That's more than I've done in a few years haha. |
Fair. I'm not sure what we should do on this so I think we can create a new discussion and revisit this once the other things are done.
Will add this to the list. |
The test are finished now. I will now issue the pull request (update: I seem to lack the permissions to do that, so please @z3t0 can you unlock master, or do this for me?), to have a clean state for master. I hope you will review it successful, Next step is propose is to merge #437 and maybe start to look at a few open ends and close them. |
Sorry, I do not understand. TIMER_PWM_PIN has already been renamed to SEND_PIN (which is the better name). Or do you mean it the other way around?
What the PR does is to make the basic functionality of the library available to a few new hardware platforms. So it should be tested using the same tests used for other, already supported hardware, simple sending and simple receiving. Or is there anything I have missed? Do you need me to rebase the PR? |
@bengtmartensson |
Can you try now? I think I've unlocked it. Ideally though, let's hold off on making changes to master until the PR is merged in? Also could you briefly outline the new proposed development method? I think it's trunk based? We can add the info to https://github.com/z3t0/Arduino-IRremote/blob/master/Contributing.md |
@z3t0 Hi, I found the reason for not beiing able to create a PR. Now its up to you to review (just have a short look, its all tested). Who should merge the 2 conflicting files? You can do it if you want, just keep the beta files. If not, tell me, then I will do the merge.
I can prepare a changed Contributing.md the next days. |
Hi Rafi, I would like to finish this to go further before my holiday. Thanks |
Hello Rafi, |
Sorry I just saw this, I fixed it. |
@z3t0 @bengtmartensson |
I'm fine with it
…On Wed., Sep. 16, 2020, 7:26 a.m. Armin, ***@***.***> wrote:
@z3t0 <https://github.com/z3t0> @bengtmartensson
<https://github.com/bengtmartensson>
Hi
I put a lot of effort and code cleanup in the current version and I think
it is worth a new minor version number - 2.7.0.
Any vetos?
Greetings
Armin
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#525 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACEVI7PS3JR5LXSJGPEXKVTSGCOF3ANCNFSM4D32YDIA>
.
|
For sure (I did not know I had veto rights??!!) Congratulations for the great work you have done the last few months :-). |
@z3t0 Thanks and happy x-mas |
Hi Armin, Thank you for all your amazing work on this project! You've given it a new life 😄 I tried to add you as an admin but could not find the option. I've created a new organization and made you an owner. The repo is now transferred there.
Please consider yourselves an "administrator" or "owner". I believe you've done a really good job, the community certainly appreciates your contributions and you are better equipped to make decisions on the future of this project than myself! As such, feel free to make any decisions as you see fit. Happy x-mas! 🥳 Thanks, |
Thanks Rafi |
Hi all!
I greatly appreciate all the effort put into this project by members at all levels, including programmers, testers and users who provide valuable feedback. Clearly, I have fallen behind quite a bit in the development and PR merging phases. However, this is just how open source projects tend to go as everyone here is a volunteer working on their own time and for free.
I would like to open up the floor to promote some of the core contributors to being authorized to directly accept PRs and add commits to the repository. This way the project can continue to grow at a faster pace and will have more than just one contributor involved in the final release process.
The only requirement is that you have spent a reasonable amount of time being visible in this project. Either by supporting other users, submitting PRs, or testing the library.
The text was updated successfully, but these errors were encountered: