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

Remove frontail by default #1933

Merged
merged 3 commits into from
Dec 18, 2024
Merged

Remove frontail by default #1933

merged 3 commits into from
Dec 18, 2024

Conversation

ecdye
Copy link
Contributor

@ecdye ecdye commented Dec 15, 2024

See #1921 for more specific details for the reasoning.

See openhab#1921 for more specific details for the reasoning.

Signed-off-by: Ethan Dye <[email protected]>
@ecdye ecdye requested a review from mstormi December 15, 2024 23:12
functions/nodejs-apps.bats Show resolved Hide resolved
openhabian-setup.sh Show resolved Hide resolved
@ecdye ecdye merged commit a805405 into openhab:main Dec 18, 2024
9 checks passed
@mstormi
Copy link
Contributor

mstormi commented Dec 18, 2024

I agree with @mherwege uninstall should be completely optional for now.
We should not remove the install option as someone to accidentially have removed it will want it back and someone like Mark who is not confident with the new builtin tool will want to keep using frontail, too.

Please add a big warning instead to pop up during frontail install that it is still useable but no longer supported.
(yeah of course thats the main point to avoid spending support on this. I have more or less been hating frontail forever)

We can revisit when either the builtin viewer improved or someone contributes a frontail alternative see #1921 .

Please add back the openhabian.conf options for unattended install, too.

@mstormi
Copy link
Contributor

mstormi commented Dec 18, 2024

The newly built in viewer, at least for now, isn't an alternative for everybody yet.
There's already first complaints on the forum similar to Mark's comment above.

@ecdye That removal was too hasty. Please next time don't merge such changes without prior consent.

Plus, for any changes, put them up for main branch but do not forward them straight away to openHAB branch.
Doing so makes them become effective immediately to any openHABian user on his/her next start of openhabian-config.
Normally, you should keep changes in main branch 'quarantine' for some weeks first before you forward them.

I've commented remove_frontail() out for now in openHAB branch as a quick stop.
Please rework that such that users are asked if they want to remove it, cross-check that it can still be (re)installed, and also
adapt NEWS.md accordingly.

@ecdye
Copy link
Contributor Author

ecdye commented Dec 18, 2024

@ecdye That removal was too hasty. Please next time don't merge such changes without prior consent.

Yes, I was a little rushed in merging that change. My apologies, I was up late last night finishing up the final touches on the zram-config fixes and got a little greedy in pushing this one in as well.

In general I think we could both do better about waiting until reviews have been given to merge things and not directly merging into main using Maintainer access. I'll look into adding some additional checks to help us both be more compliant in the future with the reviews required policy outlined in the openHAB docs.

Plus, for any changes, put them up for main branch but do not forward them straight away to openHAB branch.

Doing so makes them become effective immediately to any openHABian user on his/her next start of openhabian-config.

Normally, you should keep changes in main branch 'quarantine' for some weeks first before you forward them.

Yes, once again my apologies, I had to forward the zram-config changes and got a little carried away in adding this in as well. I recall the annoyance of having every little change we made going to the general public and all the issues it caused that made us switch to the two branch model, I'm not keen to return to that.

I've commented remove_frontail() out for now in openHAB branch as a quick stop.

Please rework that such that users are asked if they want to remove it, cross-check that it can still be (re)installed, and also

adapt NEWS.md accordingly.

I will work on that sometime in the next day or two. I plan to make it very clear that it is not a supported model anymore as I'm really done with frontail and all of the headaches it has given me over the years.

@mstormi
Copy link
Contributor

mstormi commented Dec 18, 2024

I understand you, this can happen at times and yes, I've been pretty much hating frontail ever since I had to resolve my first issue with it some years ago, too. People talked to me as if I had been the person to have selected it (it wasn't me!!) and went on to urge me fix it anytime something didn't work.

But users have no proper alternative yet so we must at least keep it available to them for some more time.
(and to be frank the 'security vulnerabilities' are no convincing argument to me, give that the OHian box normally isn't exposed to inet nor are there any other users on the box or in the LAN.)

@ecdye
Copy link
Contributor Author

ecdye commented Dec 18, 2024

I understand you, this can happen at times and yes, I've been pretty much hating frontail ever since I had to resolve my first issue with it some years ago, too. People talked to me as if I had been the person to have selected it (it wasn't me!!) and went on to urge me fix it anytime something didn't work.

I wish I had never taken the time to give it an update to make it limp along 2 years ago, because now it has just made it more of a headache but here we are.

But users have no proper alternative yet so we must at least keep it available to them for some more time.

Yeah, but I will be really annoying and if they are on openHAB 4.3+ I will make a pop up appear every time the use openhabian-config reminding them that it is not supported and has issues.

ecdye added a commit that referenced this pull request Dec 19, 2024
This reverts commit cc5489164816cac0c708029ca4b7fafb2cc4920f.

Signed-off-by: Ethan Dye <[email protected]>
ecdye added a commit that referenced this pull request Dec 19, 2024
Related #1933

Signed-off-by: Ethan Dye <[email protected]>
ecdye added a commit that referenced this pull request Dec 20, 2024
* Revert "Remove frontail by default (#1933)"

This reverts commit cc5489164816cac0c708029ca4b7fafb2cc4920f.

* Prompt user to remove frontail

Related #1933

* Update docs for deprecation of frontail

* Update wording in NEWS for clarity

---------

Signed-off-by: Ethan Dye <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants