Skip to content

Handle cluster setup #125

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.

Sign up for GitHub

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

Jump to bottom
Merged
PVince81 merged 5 commits into from
Aug 14, 2017
Merged

Handle cluster setup #125

PVince81 merged 5 commits into from
Aug 14, 2017

Conversation

@DeepDiver1975
Copy link
Member

@DeepDiver1975 DeepDiver1975 commented Jul 31, 2017 •
edited
Loading

@DeepDiver1975 DeepDiver1975 added this to the development milestone Jul 31, 2017
@DeepDiver1975 DeepDiver1975 requested a review from phisch July 31, 2017 14:10
@VicDeo
Copy link
Member

VicDeo commented Jul 31, 2017

@DeepDiver1975 tests? ;)

@DeepDiver1975
Copy link
Member Author

DeepDiver1975 commented Aug 10, 2017

This is ready to be reviewed and merged as soon as we did release 10.0.3

@PVince81
Copy link
Contributor

PVince81 commented Aug 11, 2017

This is ready to be reviewed and merged as soon as we did release 10.0.3

So does it need to wait or should it be released as part of 10.0.3 ? 😕

@DeepDiver1975
Copy link
Member Author

DeepDiver1975 commented Aug 14, 2017

So does it need to wait or should it be released as part of 10.0.3 ? 😕

should be part of 10.0.3 from my pov - I could relax the dependency and use some defensive coding ... method_exists ....

@PVince81
Copy link
Contributor

PVince81 commented Aug 14, 2017

so we should get this merged then

PVince81
PVince81 reviewed Aug 14, 2017
View reviewed changes
lib/MarketService.php
}

public function canInstall() {
if (!method_exists($this->appManager, 'canInstall')) {
Copy link
Contributor

@PVince81 PVince81 Aug 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add TODO: remove once min-version is set to 10.0.3+

PVince81
PVince81 approved these changes Aug 14, 2017
View reviewed changes
Copy link
Contributor

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 otherwise

@PVince81 PVince81 mentioned this pull request Aug 14, 2017
Merged
@PVince81 PVince81 merged commit 5ab51f7 into master Aug 14, 2017
@PVince81 PVince81 deleted the handle-cluster-setup branch August 14, 2017 12:37
@PVince81
Copy link
Contributor

PVince81 commented Aug 14, 2017

@DeepDiver1975 backport to stable10 please

@DeepDiver1975
Copy link
Member Author

DeepDiver1975 commented Aug 14, 2017

@DeepDiver1975 backport to stable10 please

there is much more stuff missing afaik .... will take care later ...

@DeepDiver1975
Copy link
Member Author

DeepDiver1975 commented Aug 14, 2017

damn ... did we backport anything?

@DeepDiver1975
Copy link
Member Author

DeepDiver1975 commented Aug 14, 2017

#131

@patrickjahns
Copy link
Contributor

patrickjahns commented Aug 14, 2017

Shouldn't this also address #123 (no update when directory not writeable)

@patrickjahns patrickjahns mentioned this pull request Aug 14, 2017
Closed
@DeepDiver1975
Copy link
Member Author

DeepDiver1975 commented Aug 14, 2017

Shouldn't this also address #123 (no update when directory not writeable)

yes - closed - thx

@DeepDiver1975 DeepDiver1975 modified the milestones: development, QA Sep 15, 2017
@patrickjahns patrickjahns mentioned this pull request Nov 13, 2017
Closed
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@phisch phisch Awaiting requested review from phisch

1 more reviewer

@PVince81 PVince81 PVince81 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@DeepDiver1975 DeepDiver1975

Labels

3 - To Review

Projects

None yet

Milestone

10.0.3 / 0.2.2

Development

Successfully merging this pull request may close these issues.

Add market:uninstall command to occ When installing a local app without enough permissions the message shown is not clear.

5 participants

@DeepDiver1975 @VicDeo @PVince81 @patrickjahns