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

DietPi-Software | Add Homer dashboard #5833

Merged
merged 21 commits into from
Nov 1, 2022
Merged

DietPi-Software | Add Homer dashboard #5833

merged 21 commits into from
Nov 1, 2022

Conversation

MichaIng
Copy link
Owner

No description provided.

- DietPi-Software | Homer: Apply suggestions from #5829
@Joulinar
Copy link
Collaborator

Do we really need the backup or is it for safety reasons only?

@MichaIng
Copy link
Owner Author

Actually I think we don't need it, but @t3dium will better know this. I already applied the suggestions we made and the config file is only created from the sample, if there is not already one. So for the config file only, no backup would be required anymore.

- DietPi-Software | Homer: Remove old assets with hash in file name on reinstalls, to not accumulate obsolete files
@MichaIng
Copy link
Owner Author

With Apache:
homer

PHP is unnecessarily installed currently, but as said, I'll change this afterwards in a dedicated PR.

Ah, favicon and application icons in assets/icons/ and assets/manifest.json is something users might change: https://github.com/bastienwirtz/homer/blob/main/docs/configuration.md
In theory we could automatically move those files, if existent, from the existing instance to the newly extracted archive, before moving it in place. But probably this is too complicated and it's fine that users can recover the files they changed from the backup.

There are a lot of assets, especially everything in resources/, with a short hash in the file name. This is one way to assure that browser cache does not cause havoc on updates. However, if we did not remove them on reinstall, they would be left while new ones with different hash/file name are added. So those are removed now, after the backup was created.

@MichaIng MichaIng requested a review from Joulinar October 25, 2022 19:30
@MichaIng
Copy link
Owner Author

MichaIng commented Oct 25, 2022

Woks well here with all webservers. @t3dium if you are fine, I'd squash all commits on merge, to assure none of the back and forth havoc we caused is merged into dev. But I'll add the sign-off for your account, which GitHub would add with a merge commit, so it is credited to you. But of course you can also open a new PR and cherry-pick my last 3 commits.

- DietPi-Software | Homer: Assure as well that obsolete assets with hash in file name do not accumulate in Homer backup
dietpi/dietpi-software Outdated Show resolved Hide resolved
dietpi/dietpi-software Outdated Show resolved Hide resolved
@t3dium
Copy link

t3dium commented Oct 25, 2022

Do we really need the backup or is it for safety reasons only?

its mainly for safety and ease of updates

- DietPi-Dashboard | Homer: Update backup and touch existing instance on reinstalls after the download has succeeded, so if download fails, the existing instance remains untouched and functional
@t3dium
Copy link

t3dium commented Oct 25, 2022

Woks well here with all webservers. @t3dium if you are fine, I'd squash all commits on merge, to assure none of the back and forth havoc we caused is merged into dev. But I'll add the sign-off for your account, which GitHub would add with a merge commit, so it is credited to you. But of course you can also open a new PR and cherry-pick my last 3 commits.

Yeah thats fine

- DietPi-Software | Homer: Offer to remove the Homer backup on uninstall
Joulinar
Joulinar previously approved these changes Oct 25, 2022
t3dium
t3dium previously approved these changes Oct 25, 2022
dietpi/dietpi-software Show resolved Hide resolved
- DietPi-Software | Homer: Add changelog, survey report support, README entry and reduce software ID to 205
@MichaIng MichaIng dismissed stale reviews from t3dium and Joulinar via 090044d November 1, 2022 19:26
@MichaIng MichaIng merged commit ca14f45 into dev Nov 1, 2022
@MichaIng MichaIng deleted the homer branch November 1, 2022 19:30
@MichaIng MichaIng mentioned this pull request Nov 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants