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

MQTT read_table_A.pl support #804

Merged
merged 1 commit into from
Dec 17, 2020
Merged

Conversation

jsiddall
Copy link
Contributor

This was written by Giles Godart-Brown

@hollie
Copy link
Owner

hollie commented Dec 16, 2020

This pull request passes the Travis check, but the result is not reflected in the status above due to a change I had to do from travis-ci.org to travis-ci.com.

The actual result of the test is here: https://travis-ci.com/github/hollie/misterhouse/builds/209211100

So CI-wise, the pull request is OK to merge.

@hollie
Copy link
Owner

hollie commented Dec 16, 2020

Hello @jsiddall,

thanks for this pull request.

Maybe a small cosmetic suggestions: in MQTT 'speak' the server is called 'broker'.

Would it make sense to keep the name of the instance in line with the commonly used term, especially in the table definition?

So instead of calling the instance MQTT_GATEWAY I think it makes sense to rename it to MQTT_BROKER.

What do you think?

Best regards,
Lieven.

lib/read_table_A.pl Outdated Show resolved Hide resolved
@jsiddall
Copy link
Contributor Author

I replaced all occurrences of gateway with broker and amended the commit.

@hollie
Copy link
Owner

hollie commented Dec 17, 2020

Thank you Giles! Merging in...

@hollie hollie merged commit e78531f into hollie:master Dec 17, 2020
@hplato hplato mentioned this pull request Dec 29, 2020
@hplato hplato mentioned this pull request Aug 22, 2023
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