Closed maniackcrudelis closed 6 years ago
Shouldn't we take advantage of this huge refactoring to adopt latest 2.7.2 standards (no sudo prefixes in every script, remove helpers)? As official apps are set as examples, I would say a clear YES :wink:
Yes you're right, I forgot some residual sudo
And, for the helpers, just I begin to work on these refactoring before the release. And my VM is not up to date...
I'm going to fix this on my side.
Feel free to edit this PR ;)
And I also forgot to recommend to implement change_url scripts from now on ;-) So... you said the PR was ready, but I guess it's not really... :wink:
I agree, it would be nice to implemente change_url scripts. But, for now, I will just make a refactoring, not any improvement. Because, we have 20 official apps, and I'm not the maintainer of these apps. So, I think it's would be great to have, at least, our apps with the last helpers and the correct shape.
So... are we ready now for reviewing? :grinning:
It's ok, we're ready ;)
Any update on this ? Is there any way we could test the update from a previous commit in the past or is there no hope at all because the old Composer install is broken anyway now ? (no idea how this all works)
People still regularly come complain on the support that roundcube is broken... Any chance that we can move forward with this ?
As far as my opinion counts, I'm okay with merging ASAP. I can try an install if that's really needed..
@alexAubin Let's take your advise as the LGTM we're missing. And, we can merged it in 3 days in the testing branch
Agreed ! Cool, thanks :smile:
Welcome to @alexAubin in the Apps Group! 🎉 More seriously, that's a pity we couldn't get a simple LGTM from the Apps Group in 3 weeks... We've got to learn from that for the other apps approval.
Problem
Solution
PR Status
Work finished
Package_check OK.
Basic tests OK.
Upgrade not tested. The previous version can't be installed.
Could be reviewed and tested.
Validation
~Medium decision~
When the PR is mark as ready to merge, you have to wait for 7 days before really merge it.~
Since this PR has been rerouted from master to testing. It passes to a minor decision.