home_assistant_noaa_tides icon indicating copy to clipboard operation
home_assistant_noaa_tides copied to clipboard

HACS support + latest noaa_coop version

Open rsnodgrass opened this issue 3 years ago • 14 comments

  • add support for HACS (plus updating README.md with installation instructions)
  • editing of README and examples
  • bug fix for self.attr missing self reference
  • error message bug fix
  • bump to latest version of noaa_coop
  • ran black, refurb, isort Python code formating/standard tools

rsnodgrass avatar Jun 27 '22 19:06 rsnodgrass

Checking on this merge.

rsnodgrass avatar Dec 22 '22 23:12 rsnodgrass

@jshufro Any chance this would get merged into your official version so I don't have to maintain a fixed version for others?

rsnodgrass avatar Nov 18 '23 19:11 rsnodgrass

@jshufro Any chance this would get merged into your official version so I don't have to maintain a fixed version for others?

Main issue I see is the breaking change from renaming the path to support HACS. Many people just clone this repo under custom_components.

jshufro avatar Nov 18 '23 20:11 jshufro

but with HACS, people can install it and get updates without having to manage git repos at all...

I think I subscribed to this issue because I wanted to use the plugin, but I'm already managing so much software that I actually need package managers like HACS (and docker, and others) to manage the sprawl.

DanceMore avatar Nov 19 '23 03:11 DanceMore

I'm not opposed to it, just weighing the pain against the benefits.

I think it'll make peoples' lives easier long term. I'll just need to post on the forums and update the readme and all that to make the breaking change extremely well documented.

Some people don't use HACS, and it will certainly make their lives harder- now they have to git pull and cp -R the noaa_tides folder out.

jshufro avatar Nov 19 '23 03:11 jshufro

I'm an SRE by trade and one of my biased opinions is "git is not a deployment tool".

You probably use git in your packaging workflows to make "Artifacts" that endusers and sysadmins install. These are ideally some form of Package Managed Object, be it .deb, .rpm, Docker images, Pypi, Rubygem, NPM, et al, or even HACS :)

Deploying directly from git is acceptable during development; heck, it's often the only choice when you're churning code and need to test it constantly... but it's hard to turn into a robust long-term solution. As you've already seen, managing paths and subdirectories can get annoying for the needs of Developers vs Deployers. It gets even more bulky and complicated if you start adding test suites, .github/ automations, or documentation that needs a toolchain to generate. Life gets very confusing when you get bug reports from someone that isn't running an "official released version number" and no git commit-ish that you can find in your history .....

DanceMore avatar Nov 19 '23 19:11 DanceMore

Python doesn't exactly need a build pipeline.

I'd prefer to have set the repo up in a hacs-compatible way from the start, but not having done that, I don't have the luxury of blindly reorganizing things without considering what might break. As an SRE I am sure you are aware.

jshufro avatar Nov 19 '23 21:11 jshufro

@rsnodgrass looking at the options for manifest.json, it seems like setting filename should remove the need to change the directory structure- can you test that out?

image

https://hacs.xyz/docs/publish/start/#hacsjson

If that doesn't work, i'd be more open to setting content_in_root and pulling the .py files up into the project root directory, which would let people simply git clone [email protected]:jshufro/home_assistant_noaa_tides.git noaa_tides in the custom_components path if they don't want to use HACS

jshufro avatar Nov 19 '23 21:11 jshufro

@jshufro since there is only a single sensor.py, the filename approach just might work. I don't know how it works with the manifest.json file.

Another solution is to fork this into a new repo jschufro/hass-noaa-tides, add that to HACS, and then add warnings to the existing one that users should switch to the "new" one.

Thank you! Look forward to getting automated notification of updates through HACS once this is done.

rsnodgrass avatar Nov 19 '23 22:11 rsnodgrass

I will test this later this week! Thanks.

On Nov 20, 2023 at 10:20 AM, <Jacob Shufro @.***)> wrote:

@jshufro commented on this pull request.

In hacs.json (https://github.com/jshufro/home_assistant_noaa_tides/pull/22#discussion_r1399581422):

@@ -0,0 +1,6 @@ +{ + "name": "NOAA Tides and Currents", + "domains": ["sensor"], + "render_readme": true, + "filename": "sensor.py"
⬇️ Suggested change

  • "filename": "sensor.py" + "filename": "noaa_coops/sensor.py"

This would keep the path structure of the repo intact so it's the least likely to break anyone's setup.

If it isn't tenable for whatever reason, we can go with what you have here, and I'll just warn anyone who cloned into custom_components/ that they need to reclone into a subpath called noaa_coops

— Reply to this email directly, view it on GitHub (https://github.com/jshufro/home_assistant_noaa_tides/pull/22#pullrequestreview-1740428797), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AAQY4XA544QMHJ3E7XCW7DDYFONO3AVCNFSM5Z7R4BMKU5DIOJSWCZC7NNSXTPCQOVWGYUTFOF2WK43UKJSXM2LFO45TCNZUGA2DEOBXHE3Q). You are receiving this because you were mentioned.Message ID: @.***>

rsnodgrass avatar Nov 20 '23 22:11 rsnodgrass

Should I be able to test this in HACS via adding the following?

Repository: rsnodgrass/home_assistant_noaa_tides Category: integration

Asking as I tried that, but it comes back with an error.

<Integration rsnodgrass/home_assistant_noaa_tides> Repository structure for master is not compliant

rjbrown99 avatar Jan 03 '24 18:01 rjbrown99

Should I be able to test this in HACS via adding the following?

Repository: rsnodgrass/home_assistant_noaa_tides Category: integration

Asking as I tried that, but it comes back with an error.

<Integration rsnodgrass/home_assistant_noaa_tides> Repository structure for master is not compliant

Try now, the repo structure should now be compliant.

rsnodgrass avatar Jan 11 '24 17:01 rsnodgrass

@jshufro see my comment above about adding a symlink from noaa_tides to custom_components/noaa_tides. This should address your earlier concern about keeping the repo structure intact for those who have some sort of manual clone/install process.

rsnodgrass avatar Jan 11 '24 17:01 rsnodgrass

The updated repo does indeed work with HACS now, thank you.

rjbrown99 avatar Jan 12 '24 05:01 rjbrown99