You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by giraffeforestg <gi...@git.apache.org> on 2015/09/13 15:15:23 UTC
[GitHub] cloudstack pull request: CLOUDSTACK-8838: Allow ensX enoX enpX enx...
GitHub user giraffeforestg opened a pull request:
https://github.com/apache/cloudstack/pull/812
CLOUDSTACK-8838: Allow ensX enoX enpX enxX format for nics in CentOS 7
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/giraffeforestg/cloudstack CLOUDSTACK-8838
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/812.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #812
----
commit e8c5ed4e3de8129895feb648066f0c29f49da63d
Author: Satoru Nakaya <gi...@gmail.com>
Date: 2015-09-13T13:13:30Z
CLOUDSTACK-8838: Allow ensX enoX enpX enxX format for nics in CentOS 7
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8838: Allow ensX enoX enpX enx...
Posted by wido <gi...@git.apache.org>.
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/812#issuecomment-139914467
Looking at the if-statement I give it a LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8838: Allow ensX enoX enpX enx...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/812
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8838: Allow ensX enoX enpX enx...
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/812#issuecomment-139874783
@giraffeforestg Thanks for picking this up! Will test it tomorrow.
FYI: You can always make changes after the PR is opened. If you'd want to change something now, you'd push a new commit to `giraffeforestg:CLOUDSTACK-8838`. You can also alter an existing commit, or overwrite it using `-f` when pushing. In other words, whatever you do to your branch `giraffeforestg:CLOUDSTACK-8838` that this PR is linked to, will end up here.
Don't get me wrong, opening a new PR is just fine too ;-)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8838: Allow ensX enoX enpX enx...
Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/812#issuecomment-139914691
Why don’t you extract the content of line 1164 (the condition) to a method, this way it is possible to create a test case and enables some java doc. Moreover, the code becomes easier to read.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8838: Allow ensX enoX enpX enx...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/812#issuecomment-140650851
LGTM, given the if conditions
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8838: Allow ensX enoX enpX enx...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/812#issuecomment-140658229
I agree with @rafaelweingartner that this can have (and deserves) a unit test maybe it can even be generalized to dynamically find nic prefixes or at least have a list of them ourside the condition. It will make the code clearer and thus better maintainable
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---