chiefwigms / picobrew_pico

MIT License
149 stars 63 forks source link

Added hook/script to run after git updates #268

Closed cgalpin closed 3 years ago

cgalpin commented 3 years ago

Take 2

This will cover the current need to update existing installations with bluetooth disabled, and a place to add additional checks in the future.

It will run on reboot if/when update_boot is true, as well as when the server is updated via the UI when on a raspberry pi.

tmack8001 commented 3 years ago

LGTM (though haven't tested it on my setup, will do that tonight just as a sanity) @chiefwigms thoughts on this approach?

chiefwigms commented 3 years ago

I don't think half that script has any value in the image setup - you're already pulling the latest git repo, and the overlay should already be good. Probably goofy to run every git pull too - if someone wants bluetooth/tilt, just update to the latest image but 🤷‍♂️

tmack8001 commented 3 years ago

@cgalpin added it into the rc.local which means it would run after every boot.

The idea is over time we can add additional "upgrade / migration" scripts into this single shell script reducing the need to continually reflash new images just to get a new nginx config, a new feature relying on OS settings, etc.

Sure the script runs after every git pull, but that isn't a problem since each added "upgrade / migration" should have a conditional "is this change already present" and just become a no-opt. I don't think we should put everything into this script, but small things sure make sense to me. If you truly don't want "new stuff" then the user would be using update_boot: False anyways as we instruct them.

cgalpin commented 3 years ago

The idea is to simply enable bluetooth so a non-technical user gets the support without having to ssh in and make changes. That's the whole point of the pi image isn't it? But thats all the script does right now - and only if needed, otherwise it's a no-op and does no harm.

What stuff is in there that isn't adding value?

tmack8001 commented 3 years ago

I straighted it out with @chiefwigms . He thought it was at the top of the file vs within the rc.local process script.