Enhancements#2
Conversation
gtgt
commented
Oct 21, 2022
gtgt
commented
- use https protocol for data url instead of http (to avoid redirect)
- make selected county id an attribute
- show all county data as attribute
- use county data index from 0
- make selected county id an attribute - show all county data as attribute - use county data index from 0
| "documentation": "https://github.com/amaximus/fire_protection_hu", | ||
| "issue_tracker": "https://github.com/amaximus/fire_protection_hu/issues", | ||
| "version": "0.0.2", | ||
| "version": "0.0.3", |
There was a problem hiding this comment.
This is not a fix, but an enhancement, so version should rather be 0.1.0.
See semantic versioning: https://semver.org/
| "updated_at": "2022-10-21", | ||
| "visit_repo": "https://github.com/gtgt/ha-fire_protection_hu", | ||
| "changelog": "https://github.com/gtgt/ha-fire_protection_hu/releases/0.0.2a" | ||
| "version": "0.0.3", |
| "documentation": "https://github.com/amaximus/fire_protection_hu", | ||
| "issue_tracker": "https://github.com/amaximus/fire_protection_hu/issues", | ||
| "version": "0.0.2a", | ||
| "version": "0.0.2", |
There was a problem hiding this comment.
See other comments on versioning.
| "documentation": "https://github.com/amaximus/fire_protection_hu", | ||
| "issue_tracker": "https://github.com/amaximus/fire_protection_hu/issues", | ||
| "version": "0.0.2", | ||
| "version": "0.0.2a", |
| attr = {} | ||
| attr["provider"] = CONF_ATTRIBUTION | ||
| attr["county_id"] = int(self._county_id) | ||
| attr["data"] = self._fdata |
There was a problem hiding this comment.
What's the point adding all/country-wide fire protection related data as attribute? It would make a lot of "noise" in the attributes.
People usually use data from their own county.
Multiple sensors can be defined if someone is interested in other county's data.
| "updated_at": "2022-09-29", | ||
| "visit_repo": "https://github.com/amaximus/fire_protection_hu", | ||
| "changelog": "https://github.com/amaximus/fire_protection_hu/releases/0.0.2" | ||
| "version": "0.0.2a", |
| "changelog": "https://github.com/amaximus/fire_protection_hu/releases/0.0.2" | ||
| "version": "0.0.2a", | ||
| "updated_at": "2022-10-21", | ||
| "visit_repo": "https://github.com/gtgt/ha-fire_protection_hu", |
There was a problem hiding this comment.
Since it's a pull request you should leave the visit_repo and change log to point to the original repo.
If you want your own version with your enhancement then it should not be a pull request, but a simple fork and then you can use references to your repo, but you'll have to add yourself your repo to HACS
| def extra_state_attributes(self): | ||
| attr = {} | ||
| attr["provider"] = CONF_ATTRIBUTION | ||
| attr["county_id"] = int(self._county_id) |
There was a problem hiding this comment.
What's the point adding the county_id as attribute? It's just an index. What's the use case for it?
| "version": "0.0.3", | ||
| "updated_at": "2022-10-22", | ||
| "visit_repo": "https://github.com/amaximus/fire_protection_hu", | ||
| "changelog": "https://github.com/amaximus/fire_protection_hu/releases/0.0.3" |
2 participants
