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

examples: mqtt_relay: Add config for individual and json topics #2975

Merged
merged 1 commit into from
Jun 23, 2024

Conversation

gdt
Copy link
Collaborator

@gdt gdt commented Jun 20, 2024

Add variables to control publishing individual topics and the json topic. Default both to on, so that there is no behavior change.

This eases use now, as one's local diff is cleaner. It is also on the path to reading these variables from a config file.

@gdt
Copy link
Collaborator Author

gdt commented Jun 20, 2024

I'm unclear on protocol, but think it's best if I recuse myself from merging my own PRs, even if this one is obviously ok and non-controversial.

@zuckschwerdt
Copy link
Collaborator

zuckschwerdt commented Jun 20, 2024

Let it sit for a few days so everyone gets a chance to comment.
Looks good to me, merge when you see fit. Be sure to "Squash and merge" and set the commit message to e.g.
Change HA script individual and json topics to optional (#2975)

@gdt
Copy link
Collaborator Author

gdt commented Jun 20, 2024

This is not an HA script. It is mqtt_relay in general. That's how i tried to label it.

Does squash and merge let you set the message? Or are you saying I should edit it and force push first?

@zuckschwerdt
Copy link
Collaborator

Oops, missed the nuance in the filename ;)
I guess this is one example script that people actually use as-is (I do), so still a thumbs up from me to improve it

The "Squash and merge" let's you change the message (and also suggests adding the issue number). It also rebases so the merge if fast-forward (no hidden merge changes).
I don't think changing the default squash merge message is possible with commits or force pushes, I think it's based on the issue title (never bothered to read up or find out).

For the actual message just order by importance even if it sounds odd. https://triq.org/rtl_433/CONTRIBUTING.html has the verbs and optional area of work. Though examples: could be a useful addition I guess.

@gdt
Copy link
Collaborator Author

gdt commented Jun 20, 2024

We should add examples:; to me the point is to orient to the area of the tree.

@gdt gdt force-pushed the feature.mqtt-topic-control branch from e68a246 to 68c0299 Compare June 20, 2024 21:36
…nal (merbanan#2975)

Add variables to control publishing individual topics and the json
topic.  Default both to on, so that there is no behavior change.

This eases use now, as one's local diff is cleaner.  It is also on the
path to reading these variables from a config file.
@gdt gdt force-pushed the feature.mqtt-topic-control branch from 68c0299 to a5ef5db Compare June 23, 2024 11:24
@gdt gdt merged commit 367378f into merbanan:master Jun 23, 2024
8 checks passed
@gdt gdt deleted the feature.mqtt-topic-control branch June 23, 2024 11:25
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

Successfully merging this pull request may close these issues.

2 participants