Skip to content

Conversation

@caberos
Copy link
Contributor

@caberos caberos commented Apr 1, 2020

#1242

  • hw billing
  • vs billing

@caberos caberos requested a review from allmightyspiff April 1, 2020 00:10
Copy link
Member

@allmightyspiff allmightyspiff left a comment

Choose a reason for hiding this comment

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

Code looks good. Just make sure all the table column names start with a capital letter. And fix the few I mentioned specifically.

@allmightyspiff allmightyspiff linked an issue Apr 1, 2020 that may be closed by this pull request
@caberos caberos requested a review from allmightyspiff April 6, 2020 14:17
Copy link
Member

@allmightyspiff allmightyspiff left a comment

Choose a reason for hiding this comment

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

Looks good. Last thing please add these commands to docs\cli\hardware.rst and docs\cli\vs.rst
Thanks.

Copy link
Member

@allmightyspiff allmightyspiff left a comment

Choose a reason for hiding this comment

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

for the failing unit tests, I'd recommend changing them so they don't require an EXACT match on the output because if you change the output, the test fails. Id recommend just checking a few key fields exist in the output like how the vs_tests/ def test_bandwidth_vs_quite(self): does it

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 94.492% when pulling 16ad4e0 on caberos:issue1242 into 8ac0957 on softlayer:master.

@caberos caberos requested a review from allmightyspiff April 8, 2020 20:30
@allmightyspiff allmightyspiff merged commit f4f403c into softlayer:master Apr 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Billing details for vsi and hardware

3 participants