YunoHost-Apps / wallabag2_ynh

Wallabag v2 package for YunoHost
https://www.wallabag.org/
GNU Affero General Public License v3.0
62 stars 14 forks source link

Testing: update to 2.4.3 #128

Closed lapineige closed 2 years ago

lapineige commented 2 years ago

125

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)

lapineige commented 2 years ago

!gogogadgetoci

yunohost-bot commented 2 years ago

:rocket: Test Badge

yalh76 commented 2 years ago

!testme

yunohost-bot commented 2 years ago

:sunflower: Test Badge

lapineige commented 2 years ago

Let's wait for #132 to be ready before merging.

About #129, if it is ready at that time maybe we can wait for it, but I am not against merging without this PR and handle those specific (rare ?) cases afterwards.

yalh76 commented 2 years ago

!bullseyetestme

yunohost-bot commented 2 years ago

[Bullseye :horse:] :stuck_out_tongue_winking_eye: Test Badge

lapineige commented 2 years ago

!testme

yunohost-bot commented 2 years ago

:carousel_horse: Test Badge

lapineige commented 2 years ago

!bullseyetestme

yunohost-bot commented 2 years ago

[Bullseye :horse:] :carousel_horse: Test Badge

lapineige commented 2 years ago

So… this needs to be merged. The package is too out-to-date. Let's finish that PHP work.

edit : @yalh76 my bad I miss-clicked. Please review the PHP version stuff, I'm not sure I did it correctly.

lapineige commented 2 years ago

!testme

yunohost-bot commented 2 years ago

:rocket: Test Badge

lapineige commented 2 years ago

Considering the time we took to get ready to merge this version upgrade, and that we have a manual fix for the old database migration https://github.com/YunoHost-Apps/wallabag2_ynh/pull/129, I would say once @yalh76 and CI are validating theses changes, we merge it right away without waiting for https://github.com/YunoHost-Apps/wallabag2_ynh/pull/129.

Then I believe we should focus on debian11 compatibility (see #135) and secondly Wallabag 2.5. If in the mid-time someone is able to fix the old database migration issue, great, but if not let's move forward as more critical tasks are on the line. Are you ok with that ? :slightly_smiling_face:

Gofannon commented 2 years ago

Considering the time we took to get ready to merge this version upgrade, and that we have a manual fix for the old database migration https://github.com/YunoHost-Apps/wallabag2_ynh/pull/129, I would say once @yalh76 and CI are validating theses changes, we merge it right away without waiting for https://github.com/YunoHost-Apps/wallabag2_ynh/pull/129.

I'm not sure about the "manual fix". Using the branch https://github.com/YunoHost-Apps/wallabag2_ynh/pull/133 can be done now but will need to be updated as the package code will evolve I think.

Maybe, it could be run each "upgrade" or "only running for a specific version" of the package : ynh_compare_current_package_version or ynh_app_upstream_version https://yunohost.org/fr/packaging_apps_helpers#ynh_compare_current_package_version

Does testing the branch https://github.com/YunoHost-Apps/wallabag2_ynh/pull/133 on my server could be useful to decide if it can be merged? Or other tests?

If in the mid-time someone is able to fix the old database migration issue, great, but if not let's move forward as more critical tasks are on the line.

Is it possible to know "when" to apply the fix? Like by checking in the database or with the "yunohost package" version. If yes, it could be putted in the upgrade script with an "if" like here https://github.com/YunoHost/example_ynh/blob/master/scripts/upgrade#L61-L76=

yalh76 commented 2 years ago

Considering the time we took to get ready to merge this version upgrade, and that we have a manual fix for the old database migration #129, I would say once @yalh76 and CI are validating theses changes, we merge it right away without waiting for #129.

I've really no opinion, I was just passing by the repo, just applying example_ynh, I've no idea what wallabag provide or do :/

lapineige commented 2 years ago

Does testing the branch #133 on my server could be useful to decide if it can be merged? Or other tests?

You would need an old database scheme I guess, so that's not really possible I'm afraid. (but you could test that it doesn't break your migration as it is supposed not to affect it)

 I've really no opinion, I was just passing by the repo, just applying example_ynh, I've no idea what wallabag provide or do :/

I'm more asking on the packaging side of the code :)

Anyway CI fails…

lapineige commented 2 years ago

I might need your help @yalh76 :

42703 WARNING /usr/share/yunohost/helpers.d/php: line 237: php-fpm7.4: command not found 42796 WARNING The new configuration broke php-fpm?

Do you understand where the current code fails ? :sweat_smile:

lapineige commented 2 years ago

!testme

yunohost-bot commented 2 years ago

Fingers crossed! Test Badge

lapineige commented 2 years ago

!bullseyetestme

yunohost-bot commented 2 years ago

[Bullseye :horse:] :rocket: Test Badge

lapineige commented 2 years ago

!testme

yunohost-bot commented 2 years ago

:carousel_horse: Test Badge

lapineige commented 2 years ago

!bullseyetestme

yunohost-bot commented 2 years ago

[Bullseye :horse:] Fingers crossed! Test Badge

lapineige commented 2 years ago

Oh, there is some improvements for bullseye compatibility ! :tada:

lapineige commented 2 years ago

!testme

yunohost-bot commented 2 years ago

:sunflower: Test Badge

lapineige commented 2 years ago

!bullseyetestme

yunohost-bot commented 2 years ago

[Bullseye :horse:] :rocket: Test Badge

lapineige commented 2 years ago

I don't understand exactly why upgrade from previous versions fails, but if I remember well such an issue happened in the past and was specific to the CI. Hence I am merging this.

Now let's work on 2.5 ! :grinning: