Minor Updates #254
Merged
Minor Updates #254
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Pull Request Test Coverage Report for Build 620
💛 - Coveralls |
Contributor
|
It's currently pretty late here. I can take a look in the AM if that works?
…On Tue, Aug 28, 2018, 10:46 PM Ha Phan ***@***.***> wrote:
Some minor updates I cherry-picked from a project I've been working on.
You have time to do a quick code review @laszlof
<https://github.com/laszlof>?
------------------------------
You can view, comment on, or merge this pull request online at:
#254
Commit Summary
- Update composer: php-coveralls/php-coveralls
- BlockStorage: setBootable, setImageMetadata, resetStatus
- Compute: keypairType
- Networking: portSecurity, routerExternal for Network
- Networking L3: Mapping external fixed IPs
- ObjectStore: Fixed typehint
- Networking SecurityGroups Ext: add filterName
File Changes
- *M* composer.json
<https://github.com/php-opencloud/openstack/pull/254/files#diff-0> (8)
- *M* src/BlockStorage/v2/Api.php
<https://github.com/php-opencloud/openstack/pull/254/files#diff-1>
(60)
- *M* src/BlockStorage/v2/Models/Snapshot.php
<https://github.com/php-opencloud/openstack/pull/254/files#diff-2> (6)
- *M* src/BlockStorage/v2/Models/Volume.php
<https://github.com/php-opencloud/openstack/pull/254/files#diff-3>
(20)
- *M* src/BlockStorage/v2/Params.php
<https://github.com/php-opencloud/openstack/pull/254/files#diff-4>
(49)
- *M* src/Compute/v2/Api.php
<https://github.com/php-opencloud/openstack/pull/254/files#diff-5> (2)
- *M* src/Compute/v2/Models/Keypair.php
<https://github.com/php-opencloud/openstack/pull/254/files#diff-6> (4)
- *M* src/Compute/v2/Params.php
<https://github.com/php-opencloud/openstack/pull/254/files#diff-7>
(19)
- *M* src/Networking/v2/Api.php
<https://github.com/php-opencloud/openstack/pull/254/files#diff-8>
(10)
- *M* src/Networking/v2/Extensions/Layer3/Models/FixedIp.php
<https://github.com/php-opencloud/openstack/pull/254/files#diff-9> (3)
- *M* src/Networking/v2/Extensions/Layer3/Models/FloatingIp.php
<https://github.com/php-opencloud/openstack/pull/254/files#diff-10>
(5)
- *M* src/Networking/v2/Extensions/Layer3/Models/GatewayInfo.php
<https://github.com/php-opencloud/openstack/pull/254/files#diff-11>
(4)
- *M* src/Networking/v2/Extensions/SecurityGroups/Api.php
<https://github.com/php-opencloud/openstack/pull/254/files#diff-12>
(5)
- *M* src/Networking/v2/Extensions/SecurityGroups/Params.php
<https://github.com/php-opencloud/openstack/pull/254/files#diff-13>
(9)
- *M* src/Networking/v2/Extensions/SecurityGroups/Service.php
<https://github.com/php-opencloud/openstack/pull/254/files#diff-14>
(6)
- *M* src/Networking/v2/Models/Network.php
<https://github.com/php-opencloud/openstack/pull/254/files#diff-15>
(8)
- *M* src/Networking/v2/Models/Port.php
<https://github.com/php-opencloud/openstack/pull/254/files#diff-16>
(27)
- *M* src/Networking/v2/Params.php
<https://github.com/php-opencloud/openstack/pull/254/files#diff-17>
(31)
- *M* src/ObjectStore/v1/Models/StorageObject.php
<https://github.com/php-opencloud/openstack/pull/254/files#diff-18>
(4)
- *M* tests/unit/Networking/v2/Extensions/Layer3/Fixtures/Router.resp
<https://github.com/php-opencloud/openstack/pull/254/files#diff-19>
(43)
- *M* tests/unit/Networking/v2/Extensions/Layer3/Models/RouterTest.php
<https://github.com/php-opencloud/openstack/pull/254/files#diff-20>
(3)
- *M* tests/unit/Networking/v2/Fixtures/port_get.resp
<https://github.com/php-opencloud/openstack/pull/254/files#diff-21>
(7)
- *M* tests/unit/Networking/v2/Models/PortTest.php
<https://github.com/php-opencloud/openstack/pull/254/files#diff-22>
(11)
Patch Links:
- https://github.com/php-opencloud/openstack/pull/254.patch
- https://github.com/php-opencloud/openstack/pull/254.diff
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#254>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABaulmsKCFHpmWhDDtfnyjudadgWH8Oyks5uVgCcgaJpZM4WQupP>
.
|
Collaborator
Author
|
No worries @laszlof. Whenever you have time. |
laszlof
suggested changes
Aug 29, 2018
| * | ||
| * @param bool $bootable | ||
| */ | ||
| public function setBootable(bool $bootable) |
Contributor
There was a problem hiding this comment.
Seems like this should have a default value (true). The name of the method insinuates that it makes the volume bootable, rather than necessarily setting a flag.
| */ | ||
| public function setBootable(bool $bootable) | ||
| { | ||
| $bootable = boolval($bootable); |
Contributor
There was a problem hiding this comment.
Casting here would be more performant. Just splitting hairs here though.
| * | ||
| * @param array $options | ||
| * | ||
| * $options['status'] = (string) The volume status. |
Contributor
There was a problem hiding this comment.
I think the = here is supposed to line up with the next 2 lines.
| */ | ||
| public function resetStatus(array $options) | ||
| { | ||
| $options += ['id' => $this->id]; |
Contributor
There was a problem hiding this comment.
Might prefer array_merge here. If they pass in an id this isnt going to overwrite it (Maybe you want that?).
| public $contentType; | ||
|
|
||
| /** @var int */ | ||
| /** @var \DateTimeImmutable */ |
Contributor
There was a problem hiding this comment.
This typehint cant be right. This is a Content-Length header, it should probably be a string (according to API spec)
https://developer.openstack.org/api-ref/object-store/#get-object-content-and-metadata
laszlof
approved these changes
Aug 29, 2018
Contributor
|
Looks good pending travis |
Collaborator
Author
|
Thank you @laszlof |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some minor updates I cherry-picked from a project I've been working on.
You have time to do a quick code review @laszlof?