Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring, backup/restore, multi-instance #28

Merged
merged 8 commits into from
May 18, 2017
Merged

Conversation

JimboJoe
Copy link
Contributor

Major overhaul to:

  • apply latest coding standards (e.g. using forthcoming new helpers)
  • add backup/restore feature
  • allow for multi-instance install
  • comply with quality level 7

We should then:

  • add this package to the community repository
  • call for test by community
  • and hopefully switch to official repository, obsoleting the existing wallabag package 👍

@lapineige
Copy link
Member

What a great PR ^^
Thanks a lot @JimboJoe ! :)

I'm going to test in a VM for the backup/restore features.
I'll also submit this to the forum, and wait for a lot of tests, as this is a crucial feature for the users.

allow for multi-instance install

Do we need this, as wallabag can handle multiple users ?

and hopefully switch to official repository, obsoleting the existing wallabag package 👍

In that case, we could keep the old app in a separated branch, in case someone wants to use it.

@JimboJoe
Copy link
Contributor Author

allow for multi-instance install

Do we need this, as wallabag can handle multiple users ?

It obviously corresponds to a very specific need, but one may need to host several wallabag instances on several domain names (hosting service, association, etc.). And as the architecture of this application is "simple" for this purpose, why not offer the most of it? :-)

I'll also submit this to the forum, and wait for a lot of tests, as this is a crucial feature for the users.

Please, before posting in the forum (and if this PR gets merged), make a PR on apps/community.json to integrate wallabag2 to the repository (it is surprisingly missing). This way people will just have to type yunohost app install wallabag2. I can help you if you want.

@lapineige
Copy link
Member

And as the architecture of this application is "simple" for this purpose, why not offer the most of it? :-)

Good point :)

make a PR on apps/community.json to integrate wallabag2 to the repository (it is surprisingly missing)

Oh yes I saw that in the past, but I forgot to send a PR :/

before posting in the forum (and if this PR gets merged)

Why before ? We can ask for some beta test for advanced users (who won't be afraid to do it via the command line, on a test machine / with backups).
Then if everything is working correctly, we submit it to the community.json list, and then we could make another announcement in the forum.

@JimboJoe
Copy link
Contributor Author

... and call again for extensive testing/use by the community to then apply to the official list ;-)
I let you steer the process 😉

@lapineige
Copy link
Member

No, I mean if this version is working fine, we merge it and apply for the official list.

@lapineige
Copy link
Member

One person reported that he have the same error with the bookmarklet than before your previous update.
He updated, tested the bookmarklet, it worked well. But after a backup + restore, it doesn't work any more.
Did you experienced the same behaviour, or should I investigate more in his case ?

@JimboJoe
Copy link
Contributor Author

I just tried and couldn't reproduce the problem.
Maybe the user backed up the package before upgrading... ?
On what media did the user report the problem to you?

@lapineige lapineige mentioned this pull request May 18, 2017
@lapineige lapineige merged commit 29ccad0 into master May 18, 2017
@maniackcrudelis maniackcrudelis deleted the code_refactoring branch November 19, 2017 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants