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

Testing #50

Open
wants to merge 53 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@maniackcrudelis
Copy link

commented Jan 28, 2019

Problem

  • This app uses master since a long time. Neglecting the testing branch
  • Global upgrade of the package.
  • Upgrade to 1.12.1
  • Add a change_url script

Solution

  • Merge master into testing, and now try to merge this testing branch to restore the process.

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 :
  • Approval (LGTM) :
  • Approval (LGTM) :
  • CI succeeded :
    Build Status
    When the PR is marked as ready to merge, you have to wait for 3 days before really merging it.

maniackcrudelis and others added some commits Oct 8, 2017

Merge pull request #42 from YunoHost-Apps/Add-upgrade-from-a-previous…
…-commit

Add upgrade from a previous commit
Create pull_request_template.md
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
Create rainloop_version
Add a rainloop_version file to fix conflict in #49
Merge pull request #49 from YunoHost-Apps/master
Merge master into testing

@maniackcrudelis maniackcrudelis requested a review from YunoHost-Apps/apps-group Jan 28, 2019

@JimboJoe

This comment has been minimized.

Copy link

commented Feb 10, 2019

Hm... shouldn't we advertise this testing before releasing it...? 🤔

@maniackcrudelis

This comment has been minimized.

Copy link
Author

commented Feb 10, 2019

If possible, it would be better to merge #48 before. So we would really have something to test with this branch.

@alexAubin

This comment has been minimized.

Copy link
Member

commented Feb 17, 2019

Idk where to say this but #36 is still relevant ... sounds like we should be installing php-curl somewhere in this app :s

maniackcrudelis and others added some commits Feb 25, 2019

@maniackcrudelis

This comment has been minimized.

Copy link
Author

commented Mar 9, 2019

Waits for #54

maniackcrudelis added some commits Mar 9, 2019

@alexAubin alexAubin referenced this pull request Mar 20, 2019

Merged

Package upgrade #54

8 of 8 tasks complete
@alexAubin

This comment has been minimized.

Copy link
Member

commented Mar 24, 2019

Randomly passing by, made a fresh install on a RPi and everything seems to work smoothly 👍

@kay0u
Copy link
Member

left a comment

Really nice work! That was not an easy upgrade :S

Show resolved Hide resolved scripts/install
Level 8=0
Level 9=0
Level 10=0
Level 4=1

This comment has been minimized.

Copy link
@kay0u

kay0u May 15, 2019

Member

Since this PR, check_process have changed, don't need these "level" anymore

@kay0u kay0u requested a review from YunoHost-Apps/apps-group May 15, 2019

@maniackcrudelis

This comment has been minimized.

Copy link
Author

commented May 22, 2019

Ready to be reviewed (finally)

@kay0u

This comment has been minimized.

Copy link
Member

commented May 22, 2019

We should wait #56

@maniackcrudelis

This comment has been minimized.

Copy link
Author

commented May 22, 2019

As #56 add fail2ban support, that means a new week of testing...

@kay0u

This comment has been minimized.

Copy link
Member

commented May 22, 2019

As you wish. But is another week for this PR (started in January) really a problem?

@maniackcrudelis

This comment has been minimized.

Copy link
Author

commented May 22, 2019

Not really a problem no, but too bad.
Let's see if #56 will be validated soon as well.

@yalh76

This comment has been minimized.

Copy link

commented May 22, 2019

Not really a problem no, but too bad.
Let's see if #56 will be validated soon as well.

Maybe enough waiting:

  • master branch provide rainloop 1.11.3 from september 2017
  • testing provide rainloop 1.12.1 from july 2018

And this PR is already the winning app PR: https://dash.yunohost.org/testings ^^

@kay0u
Copy link
Member

left a comment

Except this, everything LGTM

ynh_replace_string "$old_domain" "$new_domain$new_path" "$final_path/index.php"
else
ynh_die "Error changing the URL"
fi

This comment has been minimized.

Copy link
@kay0u

kay0u May 22, 2019

Member

Hmm something like that :

ynh_replace_string "$old_domain${old_path%/}" "$new_domain${new_path%/}" "$final_path/index.php"
cp ../sources/sso/sso.php "$final_path/index.php"
ynh_replace_string "__DOMAIN__" "$domain" "$final_path/index.php"
ynh_replace_string "__ALIASTOCHANGE__" "$final_path" "$final_path/index.php"
if [ $path_url = "/" ]; then

This comment has been minimized.

Copy link
@kay0u

kay0u May 22, 2019

Member

Same here, but less critical:

ynh_replace_string "__ROOTTOCHANGE__" ${path_url%/}" "$final_path/index.php"
cp ../sources/sso/sso.php "$final_path/index.php"
ynh_replace_string "__DOMAIN__" "$domain" "$final_path/index.php"
ynh_replace_string "__ALIASTOCHANGE__" "$final_path" "$final_path/index.php"
if [ $path_url = "/" ]; then

This comment has been minimized.

Copy link
@kay0u

kay0u May 22, 2019

Member

And here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.