Skip to content

Conversation

alexazarh
Copy link
Contributor

@alexazarh alexazarh commented Jun 14, 2016

  1. Added GetApplicationPorts command as a connected command
  2. Refactored SG creation so it will not create a SG if the values of the inbound/outbound attributes are not valid
  3. Fixed failing tests and add new tests

Not breaking


This change is Reviewable

Refactored SG creation so it will not create a SG if the values of the inbound/outbound attributes are not valid
Fixed failing tests and add new tests
@coveralls
Copy link

Coverage Status

Coverage increased (+2.0%) to 82.599% when pulling 1f2aff9 on feature/alex_70_get_app_ports into 5ce8fac on develop.

@razaba
Copy link
Contributor

razaba commented Jun 15, 2016

Reviewed 13 of 13 files at r1.
Review status: all files reviewed at latest revision, 4 unresolved discussions.


drivers/aws_shell_driver/src/drivermetadata.xml, line 3 [r1] (raw file):

<Driver Description="This driver orchestrate all the command that will be executed on AWS" MainClass="driver.AWSShellDriver" Name="AWS Shell Driver" Version="0.1.9999999">
    <Layout>
        <Category Name="Deployment">

please fix the version and then run pack.bat before checking in


package/cloudshell/cp/aws/domain/deployed_app/operations/app_ports_operation.py, line 40 [r1] (raw file):

    def _get_custom_param_value(self, custom_params, name):
        name = name.lower()

shouldn't it be in a domain service?
VMCusotomParamService?


package/tests/test_deployed_app/init.py, line 0 [r1] (raw file):
do we need to add author?


package/tests/test_deployed_app/test_operations/init.py, line 0 [r1] (raw file):
do we need to add author?


Comments from Reviewable

@alexazarh
Copy link
Contributor Author

package/tests/test_deployed_app/init.py, line [r1] (raw file):

Previously, razaba (Raz Abadi) wrote…

do we need to add author?

no because its in the testing part

Comments from Reviewable

@coveralls
Copy link

Coverage Status

Coverage increased (+2.4%) to 83.002% when pulling 0cee4c6 on feature/alex_70_get_app_ports into d1b489f on develop.

@razaba
Copy link
Contributor

razaba commented Jun 15, 2016

Reviewed 8 of 9 files at r2.
Review status: all files reviewed at latest revision, 4 unresolved discussions.


Comments from Reviewable

@razaba
Copy link
Contributor

razaba commented Jun 15, 2016

Reviewed 1 of 13 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@razaba
Copy link
Contributor

razaba commented Jun 15, 2016

:lgtm:


Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@razaba
Copy link
Contributor

razaba commented Jun 15, 2016

Reviewed 1 of 13 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@alexazarh
Copy link
Contributor Author

Review status: all files reviewed at latest revision, 2 unresolved discussions.


package/cloudshell/cp/aws/domain/deployed_app/operations/app_ports_operation.py, line 40 [r1] (raw file):

Previously, razaba (Raz Abadi) wrote…

shouldn't it be in a domain service?
VMCusotomParamService?

Done.

Comments from Reviewable

@alexazarh
Copy link
Contributor Author

Review status: all files reviewed at latest revision, 1 unresolved discussion.


package/tests/test_deployed_app/test_operations/init.py, line [r1] (raw file):

Previously, razaba (Raz Abadi) wrote…

do we need to add author?

Done.

Comments from Reviewable

@razaba razaba merged commit 3a7f044 into develop Jun 15, 2016
@razaba razaba removed the ready label Jun 15, 2016
@alexazarh alexazarh deleted the feature/alex_70_get_app_ports branch June 20, 2017 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants