esp, platformio.ini prepare for tx

olliw42 / mLRS

2.4 GHz & 915/868 MHz & 433 MHz/70 cm LoRa based telemetry and radio link for remote controlled vehicles

GNU General Public License v3.0

279 stars 58 forks source link

esp, platformio.ini prepare for tx #150

Closed olliw42 closed 3 months ago

olliw42 commented 3 months ago

@tmcadam @jlpoltrack for one (or both) of you to review and if good to merge :)

title says it

procedural notes (just to bring Tom in the loop):

I think any PR related to esp could start with "esp, " in the title :)
- since it's esp stuff one of you must approve. We started the habit to use the short lgtm to indicate approval (or one of you just merges, which is implict approval LOL)
- you will note that "merging" actually means "squash and merge". For explanation: I had set up the repo this way, since squash collapses all commits into one, otherwise we would have a pretty bloated commit history and it would be hard to follow the changes
- upon "squash and merge" you'll get a change to change the commit title. I'd like to pay a bit attention to it. Especially - following habits of ArduPilot - which I like - I like to start the title with e.g. "tools", "common", "rx", "tx", or "rx,tx", with the comment comma separated. This I really find helpfull. If this PR would be related to esp and rx code, I would have choosen a title "esp, rx, blabla".

rotorman commented 3 months ago

You might want to have a look at "Conventional Commits": https://www.conventionalcommits.org/ where the commit msg title would be in form of:

<type>[optional scope]: <description>

Especially this section might be interesting: https://www.conventionalcommits.org/en/v1.0.0/#why-use-conventional-commits

tmcadam commented 3 months ago

The change itself looks fine. We might some time need a distinction between the esp32 and esp8285 rx as well, but can deal with that later.

As the the ESP TX support will involve quite a lot of work and contribution from various people, I am wondering if we should create a new feature branch in this repo (maybe development-esp-tx). Then we can can all make PRs into that, a bit like how we used the main branch in my repo for the esp8285 rx. My feeling is that in a public repo main must always be stable and tested as someone could clone, build and fly at any time without our knowledge. It is quite difficult to maintain that stability with a lot of small PRs coming in as they can't be fully tested, so my preference would be to work in a separate branch, then when we are all happy that it is stable and tested, squash & merge that branch with main.

What are others thoughts on this?

olliw42 commented 3 months ago

The change itself looks fine. We might some time need a distinction between the esp32 and esp8285 rx as well, but can deal with that later.

you guys hit the button, or not :)

As the the ESP TX support will involve quite a lot of work and contribution from various people, I am wondering if we should create a new feature branch in this repo (maybe development-esp-tx). Then we can can all make PRs into that, a bit like how we used the main branch in my repo for the esp8285 rx. My feeling is that in a public repo main must always be stable and tested as someone could clone, build and fly at any time without our knowledge. It is quite difficult to maintain that stability with a lot of small PRs coming in as they can't be fully tested, so my preference would be to work in a separate branch, then when we are all happy that it is stable and tested, squash & merge that branch with main. What are others thoughts on this?

two thoughts:

I totally agree. A feature branch it should be. That's how git should be used. It also could be in someone others repo, but a branch here is probably preferrable. Maybe shorter esp-dev-tx, or esp-tx-dev, ....
discussion doesn't belong to here :)

olliw42 commented 3 months ago

we do have the dev-talk thread in discord, if it can be public, otherwise the dm group chat. :)

jlpoltrack commented 3 months ago

As the the ESP TX support will involve quite a lot of work and contribution from various people, I am wondering if we should create a new feature branch in this repo (maybe development-esp-tx).

I'm aligned with this.

tmcadam commented 3 months ago

The 'development' in the branch name should encourage people to treat it with caution, if they happen to stumble across it.

olliw42 commented 3 months ago

could you merge, or tell me what I should change to have it merged?

tmcadam commented 3 months ago

I've not had a chance to test this yet, but am sure have and are happy with the changes. Will merge on that basis.

olliw42 commented 3 months ago

THX!!