Skip to content
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

Maybe add extra i2c LCD support from @robaol #29

Open
grahamwhaley opened this issue Jun 7, 2023 · 3 comments
Open

Maybe add extra i2c LCD support from @robaol #29

grahamwhaley opened this issue Jun 7, 2023 · 3 comments

Comments

@grahamwhaley
Copy link
Owner

grahamwhaley commented Jun 7, 2023

Whilst perusing the forks of DSPham (something I do now and then), I spotted that @robaol had added support for a different I2C LCD in their repo (nice!). I'd happily consider adding this code if a suitable pull request were submitted @robaol :-)

For reference, the patch adds support using the same ArduinoMenu system, supporting the I2C LCD code from mathertel.

@robaol
Copy link

robaol commented Jun 7, 2023

I should be able to get to this in about 2 weeks, I hope that is ok. I am keen to support a great project.
I assume you would like me to merge into master and then start a PR?
R

@grahamwhaley
Copy link
Owner Author

Hi @robaol . Thanks for the reply! No rush here on my side - just if and when you are ready or want to :-)

I don't know how familiar you are with github PR flow and processes. Each project and user probably has their own preferences, but if it were me I'd keep those patches on your OtherLCDs branch, and send the PR from that branch. Then when the PR gets merged into my master, you can pull updates back into your master. That is, you might want to use your forks master branch to track my master branch. Make sense?

That flow also helps for if you have multiple PRs in flight for the same project - they can each live on their own branch, and your master remains 'clean', iyswim.

Apologies if you know all that already. If not, github have a number of helpful documents on making and submitting PRs etc.

A couple of thoughts/requests for your PR:

  • afaict, you are modifying files I wrote, so that code is under the GPL-3.0 license. Your PR modifications will therefore also fall under that license (inherited). Just wanted to make that clear. Better to clarify things like that earlier rather than later.
  • If you could add a comment to the commit describing what the code does a little, so that the git log shows a nice trail of edits etc. A git log in the DPSham repo will show examples of my commits for instance. You can edit the last commit in your branch with git commit --amend, which is very handy in such cases :-)
  • You might want to consider adding a Signed-off-by to your commit (git commit -s). That's not strictly necessary for this project (yet) as I've not defined what it means, but in theory let's say it means you hand over the rights to those changes to this project). It's not like I'm expecting the project to gain a huge community or commercial/industrial usage though, so I'm not that bothered right now ;-)

Thanks!

(I used to do this a lot commercially in the past, so mostly it is 2nd nature for me, but I might have to refresh my memory a little ;-) ).

@robaol
Copy link

robaol commented Sep 22, 2023

Hi Graham,
Thanks for your tips and suggestions. I will follow them as best I can but my work is rather manic 2nd half year amd so it is likely going to be December before I can give this the concentration it needs to do it properly.
Anyway, I have forgotten it and do intend to make the PR.
Best regards
Rob

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants