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

Support Services #37

Merged
merged 5 commits into from Jul 12, 2019

Conversation

@lippserd
Copy link
Member

commented Jul 8, 2019

fixes #13

@lippserd lippserd changed the title Support Services WIP: Support Services Jul 8, 2019

@lippserd

This comment has been minimized.

Copy link
Member Author

commented Jul 8, 2019

Action links and a control to switch between the cubes are missing.

class HostsController extends IdoController
{
/** @var UrlParams */
protected $params;

This comment has been minimized.

Copy link
@lippserd

lippserd Jul 10, 2019

Author Member

You don't use this variable in the controller.

public function indexAction()
{
$this->addTabs()->activate('cube/hosts');

This comment has been minimized.

Copy link
@lippserd

lippserd Jul 10, 2019

Author Member

Rename addTabs() to getTabs().

$this->buildCube($cube);
}
public function detailsAction()

This comment has been minimized.

Copy link
@lippserd

lippserd Jul 10, 2019

Author Member

Remove the detailsAction().

{
public function addTabs() {
return $this->getTabs()
->add('cube/hosts', array(

This comment has been minimized.

Copy link
@lippserd

lippserd Jul 10, 2019

Author Member

Please use short array syntax.

->extend(new DashboardAction());
}
public function buildCube($cube)

This comment has been minimized.

Copy link
@lippserd

lippserd Jul 10, 2019

Author Member

Rename buildCube() to renderCube() and make it protected.

$this->buildCube($cube);
}
public function detailsAction()

This comment has been minimized.

Copy link
@lippserd

lippserd Jul 10, 2019

Author Member

Please remove this method.

}
}
public function detailsAction()

This comment has been minimized.

Copy link
@lippserd

lippserd Jul 10, 2019

Author Member

Move the detailsAction() to the module's base controller class and rely on the protected member variable $cube.

class ServicesController extends IdoController
{
/** @var UrlParams */
protected $params;

This comment has been minimized.

Copy link
@lippserd

lippserd Jul 10, 2019

Author Member

Please remove this variable.

@@ -1,89 +1,13 @@
<?php
// Icinga Web 2 Cube Module | (c) 2016 Icinga GmbH | GPLv2
// Icinga Web 2 Cube Module | (c) 2019 Icinga GmbH | GPLv2

This comment has been minimized.

Copy link
@lippserd

lippserd Jul 11, 2019

Author Member

Don't change the copyright year here please.

}
return $cube;
public function indexAction() {

This comment has been minimized.

Copy link
@lippserd

lippserd Jul 11, 2019

Author Member

There is a newline missing.

This comment has been minimized.

Copy link
@theFeu

theFeu Jul 11, 2019

Contributor

A newline before the action?

This comment has been minimized.

Copy link
@lippserd

lippserd Jul 11, 2019

Author Member

A newline before the {.

This comment has been minimized.

Copy link
@theFeu

theFeu Jul 11, 2019

Contributor

🤦‍♂️

/** @var \Icinga\Module\Cube\Cube */
protected $cube;
abstract protected function getCube();

This comment has been minimized.

Copy link
@lippserd

lippserd Jul 11, 2019

Author Member

Please add function docs.

This comment has been minimized.

Copy link
@lippserd

lippserd Jul 11, 2019

Author Member

It is either return or get. We don't add the s.

@theFeu theFeu changed the title WIP: Support Services Support Services Jul 11, 2019

@theFeu theFeu force-pushed the feature/services branch from bdff065 to 9694850 Jul 11, 2019

@theFeu

This comment has been minimized.

Copy link
Contributor

commented Jul 12, 2019

@lippserd ready to merge?

@lippserd

This comment has been minimized.

Copy link
Member Author

commented Jul 12, 2019

There are WIP commits. Please amend/squash.

@theFeu theFeu force-pushed the feature/services branch 2 times, most recently from 9cce0e4 to fe8c2f0 Jul 12, 2019

@theFeu theFeu force-pushed the feature/services branch from fe8c2f0 to 2f46b65 Jul 12, 2019

@lippserd lippserd merged commit 2cfb4c8 into master Jul 12, 2019

@lippserd lippserd deleted the feature/services branch Jul 12, 2019

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