Upgrade dolibarr version, install require dependencies, upgrade php version

YunoHost-Apps / dolibarr_ynh

Dolibarr ERP & CRM is a modern software to manage your organization's activity. This is an integration of Dolibarr in YunoHost

https://www.dolibarr.org/

GNU Affero General Public License v3.0

13 stars 19 forks source link

Upgrade dolibarr version, install require dependencies, upgrade php version #36

Closed FHenry closed 4 years ago

FHenry commented 4 years ago

Action

Upgrade dolibarr version to lastest : 12.0.2
Install php dependcies required by dolibarr normally check by dolibarr/htdocs/install/check.php (inpired by https://github.com/YunoHost-Apps/rainloop_ynh/blob/testing/scripts/_common.sh)
Upgrade php version (inspired by https://github.com/YunoHost-Apps/rainloop_ynh/commit/f92573b80d423f99e302c7dee2cd6c59eb6a3ea8)

PR Status

[X] Code finished.
[x] Tested with Package_check.
[x] Fix or enhancement tested.
[X] Upgrade from last version tested.
[X] Can be reviewed and tested.

FHenry commented 4 years ago

@mastereur Hello, could you take me in touch about this PR ? I've seen that you've directly push on testing branch some update I've suggested on this PR (before your push) without merging my PR. I'm an involved Dolibarr developper since more than 5 years now and I want to help your project (I've send a banck transfer a month ago to thank YunoHost team for their works). Let me know if I miss something into ynh framework into this PR.

mastereur commented 4 years ago

@FHenry Hello, To tell the truth, I resumed maintenance of the package this week because I am still waiting for 2 or 3 minor versions before switching to a major version of Dolibarr. The fact is that this time, I did the developments and the Pushs, concerning issues and optimization of the package, on the test branch before looking at the presence of Pull Request. But it appears that the modifications you made are at 90% the same as I did. So it's a matter of circumstance and a mistake on my part not to have paid attention to the requests...

Please continue like this because it allows you to have a good package.

mastereur commented 4 years ago

And if it's possible for you to also do some testing to approve the changes.

FHenry commented 4 years ago

Hello @mastereur , thank for you answer. I review again my PR, now it seems to be more cosmetics than a real upgrade, but anyway, I learn how you've done and I realy like the process deploiement. I also have a lots of questions. I don't know if it the good place, and for sure you can answer me RTFM, I deserve it, but anyway I try :

Why in manisfest.json there is into service section php7.0-fpm rather than 7.3-fpm ? (I check in other ynh package and it's always the case)
- Why in scripts/install "ynh_add_fpm_config --usage=medium --footprint=medium --phpversion=$YNH_PHP_VERSION --package="$extra_php_dependencies" is better than "ynh_install_app_dependencies $pkg_dependencies" (Same I've check on other package and it's done differently) ?

mastereur commented 4 years ago

Hello @FHenry , I asked myself the same questions while looking at other packages. I based myself on several packages like example_ynh, ttrss_ynh or nextcloud_ynh, and there are always differences.

For manifest.json, I left it at service 7.0-fpm as on all packages, I don't know what it's for. I think it has something to do with a systemd service (https://github.com/YunoHost/example_ynh/blob/master/scripts/install#L192)

For the PHP dependencies, since now the generation of /etc/php/[version]/pool.d/[app].conf is dynamic, I finally kept only 1 line where we fill in the dependencies with ynh_add_fpm_config.

I confirm that this last point is more of a cosmetic one.