-
Notifications
You must be signed in to change notification settings - Fork 60
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
feat(upower): add new formatting properties #441
feat(upower): add new formatting properties #441
Conversation
Thanks for the PR. This is definitely a good addition. There are a few small bits that I think need addressing before this goes in: This is a new feature, not a bug fix :) Could you amend your commit message to reflect that please so it gets added to the changelog correctly. Something like: I don't think adding Arguably the
I'm not sure whether the combine property should be instead of or in addition to the separated ones? Would people ever want to show one and not the other? I'm open to feedback on that one. Once that's all done I'll re-review and update the docs page. Let me know if you need any help with any of the above :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Changes requested above]
Yeah, |
655f9ae
to
4a717d0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking nearly there, cheers for your work on this. Just one minor adjustment I think, then I'm happy with it.
Are you able to squash your commits down into a single one which matches the PR title? I can give a hand with that if not.
0e9559d
to
332e6b5
Compare
Should be done now |
Cheers, lgtm. I'll update the docs to add in the relevant section & merge in the next few days. |
2f5604d
to
cc92c65
Compare
cc92c65
to
76a6816
Compare
Adds parsing in format for properties other than {percentages} π΅βπ«
Resolves #440