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

Fix/minor improvements #30

Merged
merged 1 commit into from
Jan 17, 2024
Merged

Fix/minor improvements #30

merged 1 commit into from
Jan 17, 2024

Conversation

zkygr
Copy link
Contributor

@zkygr zkygr commented Dec 3, 2023

Hello,

i updated the example in the readme, because the name of the role was wrong.
There was an underscore instead of a dash.

I also run in the issue that the mqtt part failed because my tasmota_mqtt_port was an integer and not a string and i got no feedback because no_log is always true. Now the user can switch this to false.

Copy link
Owner

@tobias-richter tobias-richter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zkygr thank you for your contribution. I disagree on the role name because it matches the ansible galaxy namespace and the ansible galaxy specs.
But proove me wrong :)

Sp please revert that change and we are good to go.

README.md Outdated Show resolved Hide resolved
@zkygr
Copy link
Contributor Author

zkygr commented Dec 7, 2023

Hello, and thanks for your fast reply :)

You are absolutly right, i fucked up my import statement in the requirements.yml:

  - name: tobias-richter.tasmota
    src: https://github.com/tobias-richter/ansible-tasmota
    version: 1.10.0

I corrected the readme changes in the squash commit.

@zkygr
Copy link
Contributor Author

zkygr commented Dec 7, 2023

Should this role actually be executed on Tasmota devices? Since only the API is being used, it is more likely to be outsourced to localhost, right? That would be the 2nd fixup commit.

@zkygr
Copy link
Contributor Author

zkygr commented Dec 7, 2023

If the changes are okay with you, i would squash both commits.

Copy link
Owner

@tobias-richter tobias-richter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is my playbook:

- hosts: tasmota
  become: no
  gather_facts: no

  roles:
    - tobias_richter.tasmota

so no need to delegate to localhost

README.md Outdated Show resolved Hide resolved
@tobias-richter tobias-richter merged commit e7a6d58 into tobias-richter:master Jan 17, 2024
1 check passed
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