search

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

Normalization from example_ynh and some reworks #16

Closed mastereur closed 5 years ago

mastereur commented 5 years ago

Packaging reworked to match the latest example_ynh standard. Some improvements for the execution of installation and upgrade scripts.

alexAubin commented 5 years ago

Thank you for all the work ! :tada:

kay0u commented 5 years ago

:-O I was about to do it! Thank you!

kay0u commented 5 years ago

I'm sorry I always ask for something new on this PR. If I'm too boring, tell me and I'll make the changes myself.

mastereur commented 5 years ago

No, on the contrary, it's good to have a discussion. It gives you a good final code.

mastereur commented 5 years ago

With the new major version, I think we have to wait because there are always some mistakes. We will do it for a version 10.0.X

kay0u commented 5 years ago

I will soon push an update normally ready to merge, I will drop the support of the old versions because it has been tagged as non-working and after some tests..... The old versions did not work:

https://github.com/YunoHost/apps/blob/84221aa48508026ee849cec2447c8663ac9118c2/apps.json#L396

mastereur commented 5 years ago

Great! Thank you for your work. I didn't understand the importance of the weight of the messages, the page https://yunohost.org/#/packaging_apps_helpers is unavailable

kay0u commented 5 years ago

For me, it's ready to be merged, I just get a warning sometimes:

configuring ldap Warning: yunohost.hook - [890.1] [WARN] ldap user update ended with error

But it's still working. If you have any idea how this problem can be avoided you are free to solve it...

Look at my code and tell me if we can merge it.

I didn't understand the importance of the weight of the messages, the page https://yunohost.org/#/packaging_apps_helpers is unavailable

~https://yunohost.org/#/packaging_apps_helpers_en~ this doc is broken for this helper (new issue opened here: https://github.com/YunoHost/issues/issues/1374), you can find it here: https://github.com/YunoHost/yunohost/blob/ac7102a0caee9c6d2d8808ef4b345edfbdbb54e8/data/helpers.d/logging#L189 :-)

The weight define the proportion of the next action in the progress bar. The more the weight for an action is big, the more the progress bar will.... progress... at the end of this action (the end of an action is defined with the next call of ynh_script_progression).

The parameter --time is for the packager, with this one, the script will display the time taken by the previous action, so you can fill weights given value when you execute scripts.

mastereur commented 5 years ago

I just understood the error "ldap user update ended with error" when adding users, the script does not take into account the fact that the administrator is also a user and have his account already created. This is not a problem because the administrator is created during installation (step 2) it's a dolibarr script problem not yunohost implementation.

Otherwise, yes, the code is clean and ready to be merged

kay0u commented 5 years ago

That was I thought too.

Before merging it, I would like to know if you want to be added to the maintainer of this repo?

You'll get permissions to commit/merge in this repo.

mastereur commented 5 years ago

yes why not. I will be a dolibarr user so having an updated version is one of the objectives. :)

kay0u commented 5 years ago

Thank you for this awesome work!

You just need to know that our convention is to have branches master with the last stable version and testing where you test new things like upgrade app...