search

YunoHost-Apps / paheko_ynh

Paheko package for YunoHost
https://paheko.cloud
GNU Affero General Public License v3.0
3 stars 4 forks source link

Fix data upgrade #33

Closed rodinux closed 9 months ago

rodinux commented 10 months ago

Problem

Solution

PR Status

Automatic tests

Automatic tests can be triggered on https://ci-apps-dev.yunohost.org/ after creating the PR, by commenting "!testme", "!gogogadgetoci" or "By the power of systemd, I invoke The Great App CI to test this Pull Request!". (N.B. : for this to work you need to be a member of the Yunohost-Apps organization)

rodinux commented 10 months ago

!testme

yunohost-bot commented 10 months ago

:carousel_horse: Test Badge

rodinux commented 9 months ago

The PR should target testing, not master x_x ...

Yes I am sorry, my branch testing is very in advance with future release 1.3.0.... I have made a try on the branch fix-testing

rodinux commented 9 months ago

!testme

yunohost-bot commented 9 months ago

:v: Test Badge

rodinux commented 9 months ago

I think it is Ok to merge this PR first on master, sorry for all these commits...

rodinux commented 9 months ago

!testme

yunohost-bot commented 9 months ago

:rocket: Test Badge

rodinux commented 9 months ago

!testme

yunohost-bot commented 9 months ago

:stuck_out_tongue_winking_eye: Test Badge

rodinux commented 9 months ago

!testme

yunohost-bot commented 9 months ago

Meow :cat2: Test Badge

rodinux commented 9 months ago

!testme

yunohost-bot commented 9 months ago

:rocket: Test Badge

rodinux commented 9 months ago

Is it all right for you @alexAubin ? I need a reviewer please. I needed to keep a file config.local.user.php where users are suppose to edit some personal configurations, I think the better solution was to move this file on the keep folder data . To do this I have try use some if ynh_compare_current_package_version --comparison lt --version 1.2.11~ynh1; then but it did not work so I do like this in the upgrade

if [ -f "$install_dir/config.local.user.php" ]; then cp -a $install_dir/config.local.user.php $tmp_data_backup/ fi

I think I can now push this PR

alexAubin commented 9 months ago

My review is still the same as somewhere else : I don't understand all those branches :

So my position is still : all of this should be clarified, there should be a single PR testing -> master with no conflicts, and then can get into the actual reviewing

rodinux commented 9 months ago

My review is still the same as somewhere else : I don't understand all those branches :

* testing seemed to be on the verge of being mergeable, but there are many conflicts between testing and master because of stuff that got merged into master without going through testing

The reason is the bot of the release found a new version 1.3.0 on the list, but this version 1.3.0 was unstable, just a release candidate and each weeks there was a new candidate release (actually 1.3.0-rc-12) fixing bugs. You can see this here: https://fossil.kd2.org/paheko/wiki/?name=Changelog Now the developer put this last version on production and continue fixing few little bugs, but the version is still not stable, it is really a big new release with a lot of changes.

* there's this branch which fix the "data upgrade", which as far as i understand from the title is about the temporary copy of data to /tmp/whatever ... but the branch does target `master` and not `testing`

Yes, I have realized we did not have a clean process to install correctly this app, I wanted to fix it also on master branch

* .. yet there is also "fix-testing" which .. does the same thing as this branch, but targets testing ...? I have no idea why the "fix-testing" branch exists in the first place

This branch was here to fix (like a patch) the process for have a clean upgrade, working on the testing branch make me realize the process wasn't clean, some times the upgrade works but creating a duplicated folder data, or with old files that must been removed on the upgrade process...

* and yet `testing` actually already contains some tweaks about the "data upgrade" ...

It is why I am working in a fix-testing , but you are right, I can also directly work on the testing branch, but it makes a week I work some time on the fix-testing branch and it is better for me to test it before use directly the testing branch...

So my position is still : all of this should be clarified, there should be a single PR testing -> master with no conflicts, and then can get into the actual reviewing

Sooo, you prefer just wait for the testing branch been resolved first, and then push the the testing on master, it logical. I have had to upgrade the master branch without testing because there was a upgrade of the version 1.2.11 wich is the stable version, the 1.3.0 will be stable early...

rodinux commented 9 months ago

Ok, you are right, I will remove this branch with this PR and just fix the testing branch, now I can say it works nice with the new release... I have to explain you, if I also wanted to do something in the master branch, it is because I realize the process upgrade have duplicate a folder data and files, since a bit of time, something was wrong

rodinux commented 9 months ago

So my position is still : all of this should be clarified, there should be a single PR testing -> master with no conflicts, and then can get into the actual reviewing

The problem is, I have done changes on the master branch to fix bugs for the upgrade script, and the script upgrade is not so good ! I think not all the files are restored with this script, only the DB but the files in data folder... I am a little scary about if someone done an upgrade ans loss files...

The last message (from 18/09/2023) of the developer was something like this:

Paheko.cloud est passé à la version 1.3.0, ce qui a permis d'identifier et corriger des dizaines de bugs, et corriger des petits soucis d'ergonomie.

Avec 5000 associations qui l'utilisent, on a vite fait d'avoir beaucoup de bugs qui remontent 😄

Où est passée la RC11 ? et bien je l'ai publiée il y a quelques jours mais j'étais tellement occupé que je n'ai pas eu le temps de l'annoncer 😄

Voici donc la RC12 qui contient plein de bugs corrigés, et aussi :

  • le module "modèles d'écritures" ajoute maintenant un sélecteur au dessus de la saisie d'écriture, pour utiliser un modèle plus facilement
  • refonte de l'interface de gestion des extensions, qui était un peu complexe / pas très claire à utiliser à mon sens
  • ajout d'une page d'informations sur chaque extension, qui indique clairement les restrictions d'accès, et si l'extension ajoute un menu ou un bouton sur la page d'accueil
  • ajout d'une page pour voir l'utilisation de l'espace disque d'un module (et la possibilité de supprimer les données)

Dans une semaine ou deux, la 1.3.0 stable sera publiée, le temps de voir si d'autres bugs surgissent sur Paheko.cloud 😄

So I can consider it is possible to push on master or I wait for the "real" stable version ??

alexAubin commented 9 months ago

So I can consider it is possible to push on master or I wait for the "real" stable version ??

Imho, the rule of thumb is that we should focus on delivering actual stable versions, not beta/rc versions. That doesn't prevent from preparing those in a testing PR though.

rodinux commented 9 months ago

So I can consider it is possible to push on master or I wait for the "real" stable version ??

Imho, the rule of thumb is that we should focus on delivering actual stable versions, not beta/rc versions. That doesn't prevent from preparing those in a testing PR though.

Indeed, well the testing branch is this major release which must be stable on one or twoo weeks, I prefer wait. But for the stable version of this package (based on the last stable release 1.2.11), it would be nice to push this PR wich fix the process of script upgrade... really

alexAubin commented 9 months ago

In that case merge it, i dunno what to say, but you're still adding new commits just now so it's not clear if it's stable enough to be reviewed or not ...

But my point is that wether your merge this or not, your should also resolve conflicts between testing and master for the other PR to be testable and reviewable...