Normalization from example_ynh

[maniackcrudelis]

Problem

• Global upgrade of the package.

PR Status

• Code finished.
• Tested with Package_check.
• Fix or enhancement tested.
• Upgrade from last version tested.
• Can be reviewed and tested.

Validation

---

Minor decision

• Upgrade previous version :
• Code review : Kay0u
• Approval (LGTM) : Kay0u
• Approval (LGTM) : JimboJoe
• CI succeeded :
  
  When the PR is marked as ready to merge, you have to wait for 3 days before really merging it.

[JimboJoe]

LGTM 👍

[kay0u]

LGTM and code review

[kay0u]

Will be merged in 3 days

[maniackcrudelis]

I'd like to have a feedback from @Josue-T before merging, since he's the maintainer of this app.

[Josue-T]

I'd like to have a feedback from @Josue-T before merging, since he's the maintainer of this app.

Well, I would like to know why restore each file manually. Because just calling ynh_restore do everything.

[maniackcrudelis]

Because with ynh_restore_file we know exactly what is restored and where in the script.
Always the same idea, to keep the scripts clear and readable, and to avoid any hidden behaviors.

[Josue-T]

Because with ynh_restore_file we know exactly what is restored and where in the script.
Always the same idea, to keep the scripts clear and readable, and to avoid any hidden behaviors.

Well, I don't really like the idea to just add some code for nothing. The helper ynh_restore was made exaclly for that, so I don't see the reason not use it...

[maniackcrudelis]

Because we have to keep our script easy to read, easy to modify, easy to copy.
I had myself trouble with that, not knowing which files was actually restored while working on this PR.

That's always the same idea, and you know that since we already discussed that many time. Our app scripts should be the most universal as possible, to be easy to maintained by everyone, not only you or the maintainer of a given app (at this moment).
You're the maintainer of this package, for now. I don't know tomorrow if you'll still be here to do it. Neither do I. Anyone should be able to understand your package and maintained it.

[JimboJoe]

Well, I understand the general idea, but in that case, as the ynh_restore is designed to restore what has been backed up (with a successful backup process), what could go wrong...? 🤔

[maniackcrudelis]

That isn't about going wrong. I don't think that helper could go wrong.
That's about knowing what's going on, where in the script. Especially here, to know which files are restored, where and when. To be clear about what's going on, that's all. As usual.

Always the same idea... When reading a script, you should know exactly what's going on, step by step.

[JimboJoe]

Hm yes, but if you want to know precisely what files are backed up, I don't see it as too much hassle to look at... the backup script? 😇
Then, every backed up files are restored when using the ynh_restore helper. Am I missing anything else?

[maniackcrudelis]

I'm fed up of those fights...
My goal is clear, and didn't change since I'm here working on apps. I'm trying to make apps packaging as simple as possible for anyone who want to try it. As clear as possible. And especially, I'm trying to make packages resilient to time, especially to often change of packagers.

So that I, as I always did, try to avoid any factorization that would hidden some important behaviors, try to avoid what looks simple to the main packager but will be a difficulty for the one who will succeed.

So always the same thing...

Anyway, official apps will soon (not enough...) be done. And I won't have to do that again.
You want to do as you want, and don't care about others, don't care about what will happen after you? I fucking don't care! I don't use synapse! I'm not the boss here, do as you want. That's not my problem.
But if you want to do as you want, don't expect me to fix anything or to be here to check anything. If you want guys to play solo, do it, I won't.

[JimboJoe]

Well, I really don't understand why your taking that discussion like this... 😰
It's always a matter of balancing factorization vs ease of maintenance, and in some ways, factorization can ease maintenance, that's why you put that HUGE amount of efforts over years to build and maintain handy and powerful helpers (:heart:).
Then, can we agree that we can have different point of views on what helper is useful or not? For instance, you seem to eventually dislike ynh_restore, whereas ynh_setup_source suits you (and clearly you factorize the archive source in a separate app.src file).

You want to do as you want, and don't care about others, don't care about what will happen after you

Here you know that's totally false and unfair, as we're all giving time and efforts for the same goal...

So... let's say there will always be debates on what's better for the community, the maintainers, etc.... and that's for the good! 😉

So on this precise point: you have my opinion (and @Josue-T's one), and I certainly won't fight for it, as I'm not the maintainer either, but as part of the App's group, I respectfully shared my opinion... and you're the one having invested the most effort into that project, which I totally respect, and for which and I'll never be thankful enough 👍

[Thatoo]

I'm sorry to read such fight in here.
I believe @maniackcrudelis that your fight is understood and approved by everybody in here and I'm sure @JimboJoe and @Josue-T value your contributions and effort.
However, reading the page doesn't give the feeling that none of the two "want to do as they want, and that they don't care about others"... I guess there is only a difference of understanding on the meaning and call of one particular function. I am not a computer engineer and can't contribute to code but I can more or less understand these subject. So for what it worth, from an external point of view, I feel that both point of view are equally understandable, one being faster to understand (more precise) for the reader but being heavy (longer to read) , the other one being lighter to read, faster to read but forcing the reader to dig a bit his/her audit of the code by understanding a shared function within yunohost packages.
The difference is quiet thin I believe and I don't think it worth a fight.
Have a good day and thank you all for your efforts and this great work.

[kay0u]

To merge or not to merge, that is the question

[maniackcrudelis]

I'm done working on that PR.
Do whatever you want with it.

[Josue-T]

For me it's OK now.

[maniackcrudelis]

Not for me...
I won't accept the High Quality tag anymore with this restore file...

[Josue-T]

I won't accept the High Quality tag anymore with this restore file...

You can remove synapse of the High Quality app if you wan't...

But I still don't understand why you accepted this code 1 year ago...

[maniackcrudelis]

I indeed missed that in my initial review a year ago, in a huge diff of the package, and especially ynh_restore was already used so I probably missed its usage.
https://github.com/YunoHost-Apps/synapse_ynh/pull/34/files?file-filters%5B%5D=No+extension&owned-by%5B%5D=#diff-68321f788a23b70a33d17888e31bf3ef

But, I didn't missed it this time, while I was actually working on this app.
So, the fact that I missed it a year ago doesn't change anything about what I already told here and in a lot of places around github, the forum, the xmpp rooms and even probably our meetings.

So, nothing new...
Still and always the same fucking idea, that I repeat over and over...

[Josue-T]

Well, what does we do this PR...

[Josue-T]

Proposing to merge this soon.

[JimboJoe]

Agreed! As we need to move forward, we can debate whether or not to remove the "quality app" label in a separate thread.