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.

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

#46 and #48 fix #60

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

#46 and #48 fix #60

wants to merge 3 commits into from
+2 −10

Conversation

adityawbwce
Copy link
Contributor

Fixes:

  1. Server title and hostname empty on create Server title and hostname empty on create #46
  2. Refine IP addressing choices Refine IP addressing choices #48

@adityawbwce
Update configOptions.php
4ff99b4 
Issue 48 - Refine IP addressing choices
@adityawbwce
Update vmManager.php
e3ffc77 
 Refine IP addressing choices UpCloudLtd#48
@adityawbwce
Update vmManager.php
6c8f42e 
 Server title and hostname empty on create UpCloudLtd#46
scop
scop requested changes Nov 20, 2024
View reviewed changes
modules/servers/upCloudVps/lib/configOptions.php
@@ -37,13 +37,6 @@ public function configs(){
];
}

private function getNetworking()
Copy link
Member

Choose a reason for hiding this comment

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

There is a commented out call to this function a few lines above, on line 36. Let's remove that too.

modules/servers/upCloudVps/lib/configOptions.php
@@ -78,7 +71,6 @@ private function createGlobalConfigurableOptions($product)
$this->createLocationFields($groupId, $zones, $currencyId);
$this->createTemplateFields($groupId, $pomTemplates, $currencyId, $currencyCode);
$this->createBackupFields($groupId, $this->getBackups());
$this->createIPFields($groupId, $this->getNetworking());
Copy link
Member

Choose a reason for hiding this comment

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

After removing this, the createIPFields function is no longer used, let's remove the function itself too (line 87 below).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scop scop scop requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@adityawbwce @scop