add support for local edit#6
add support for local edit#6naja7host wants to merge 2 commits intoblesta:masterfrom naja7host:patch-1
Conversation
| { | ||
| // Set fields to update locally | ||
| $service_fields = ['thesslstore_token', 'thesslstore_order_id', 'thesslstore_fqdn']; | ||
| foreach ($service_fields as $field) { |
There was a problem hiding this comment.
Formatting is off here with white space usage. You should use spaces rather than tabs (1 tab = 4 spaces).
| return $response; | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
You have added the same methods twice.
| } | ||
|
|
||
| /** | ||
| * Edits the service on the remote server. Sets Input errors on failure, |
There was a problem hiding this comment.
This comment is inaccurate. You only edit fields locally and you never set errors. Careful with copy paste.
| * - value The value for this key | ||
| * - encrypted Whether or not this field should be encrypted (default 0, not encrypted) | ||
| * @see Module::getModule() | ||
| * @see Module::getModuleRow() |
There was a problem hiding this comment.
You never use these two functions you are referencing.
| * @return array A numerically indexed array of meta fields to be stored for this service containing: | ||
| * - key The key for this meta field | ||
| * - value The value for this key | ||
| * - encrypted Whether or not this field should be encrypted (default 0, not encrypted) |
There was a problem hiding this comment.
While 'encrypted' is a valid index to set, so do not set it so this comment is inaccurate.
|
|
||
| $fields = new ModuleFields(); | ||
|
|
||
| // Create domain label |
There was a problem hiding this comment.
I know the rest of this file does not adhere to it, but we try to have a maximum line length of 120 characters.
|
You can edit the merge us you want and correct the errors. It was just a quick fix to resolve some interne cases . |
|
@naja7host are you able to make the modifications suggested by @JReissmueller ? |
|
i will try to see it tonight. and i will send another merge |
|
Just a reminder that the requested changes haven't been made for this issue yet. |
3 participants
No description provided.