search

YunoHost-Apps / shaarli_ynh

Shaarli package for YunoHost
GNU General Public License v3.0
20 stars 13 forks source link

Testing / fix permissions #77

Closed lapineige closed 2 years ago

lapineige commented 2 years ago

This reverts commit ae6795704864e061bf5a5133ffd4ddd375c676e2.

CSS is broken on new install, reverting before we find a solution for this.

lapineige commented 2 years ago

Oh, dammit, I can't merge without a review… @ericgaspar can you do it please ?

Then I'll investigate what broke the CSS in your PR… After a first look, I really have no idea…

lapineige commented 2 years ago

ping @ericgaspar :up: :)

alexAubin commented 2 years ago

Ugh this looks like a really brutal way to solve the issue ... Having a look at the changelog, I can't see anything remotely related to the CSS unable to being loaded ... It would really be better to investigate what's happening exactly, for example using Firefox's debugger ...

lapineige commented 2 years ago

It would really be better to investigate what's happening exactly, for example using Firefox's debugger ...

That's sure, but having not knowledge or any clue about what would cause the issue (i would never guessed the issue you're raising) and limited time for that, I thought the best solution to solve what was a big breaking bug (all new installations are broken) was that. That would not create any significant side effect and would temporarily solve the issue while we could investigate what went wrong… To be clear, I'm not depreciating the quality of @ericgaspar pull request, and I couldn't even find any issue inside it. That was just the easiest and quickest way to solve a major breaking change (which appears to be a side effect) from my perspective.

lapineige commented 2 years ago

And if we don't find a fix quickly, I would still prefer that option.

alexAubin commented 2 years ago

!testme

yunohost-bot commented 2 years ago

Alrighty! Test Badge

alexAubin commented 2 years ago

So i wasn't able to reproduce the "no CSS" issue while installing the current version (0.12.1~ynh2)

Nevertheless there's definitely something funky with the permissions, so I force-pushed some changes to try to fix them

Feel free to try installing from this branch with yunohost app install https://github.com/YunoHost-Apps/shaarli_ynh/tree/testing

lapineige commented 2 years ago

I'm reproducing the issue on a fresh install of that branch :(

alexAubin commented 2 years ago

I'm reproducing the issue on a fresh install of that branch :(

tealc_indeed.gif ... me too, there was an issue with www-data not being able to access the corresponding assets

Should be fixed in previous commit

lapineige commented 2 years ago

I have a blank page now :sweat_smile:

lapineige commented 2 years ago

I don't know what to do, I'm not comfortable about leaving the app broken for a long time (it's not possible to install it for a month now), yet I don't want to rush anyone to find a solution to fix it. Should we revert the PR ?

alexAubin commented 2 years ago

@lapineige : sorry for the delay ... the blank page issue should be fixed now, can you try again ?

lapineige commented 2 years ago

No need to apologies :)

This solves the issue ! :tada:

Great, thanks for your work :smiley:

edit: oh, by the way I can't merge it alone (need a review). Am I not supposed to be able to do it, as maintainer of this repository ? :sweat_smile:

alexAubin commented 2 years ago

Great ! Let's go then !