Skip to content
This repository was archived by the owner on Aug 31, 2021. It is now read-only.

Conversation

frisky-dis
Copy link
Contributor

What this PR does / why we need it: This PR adds RTS interfaces.

Special notes for your reviewer: NONE

Release noteNONE

@coveralls
Copy link

coveralls commented Jun 29, 2018

Pull Request Test Coverage Report for Build 140

  • 308 of 742 (41.51%) changed or added relevant lines in 16 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-3.4%) to 62.348%

Changes Missing Coverage Covered Lines Changed/Added Lines %
openstack/rts/v1/stackresources/results.go 26 28 92.86%
openstack/rts/v1/stacktemplates/results.go 5 9 55.56%
openstack/rts/v1/stacks/errors.go 0 9 0.0%
openstack/rts/v1/stackevents/requests.go 8 17 47.06%
openstack/rts/v1/stackevents/results.go 29 42 69.05%
openstack/rts/v1/stackresources/requests.go 24 51 47.06%
openstack/rts/v1/stacks/utils.go 36 82 43.9%
openstack/rts/v1/stacks/template.go 29 86 33.72%
openstack/rts/v1/stacks/requests.go 68 149 45.64%
openstack/rts/v1/stacks/environment.go 0 83 0.0%
Totals Coverage Status
Change from base Build 128: -3.4%
Covered Lines: 3335
Relevant Lines: 5349

💛 - Coveralls

@Savasw
Copy link
Contributor

Savasw commented Jul 1, 2018

@niuzhenguo Can you please help update if the coverall result would be blocker for merge. Also suggest what steps could possibly help resolve it.

@niuzhenguo
Copy link
Member

@Savasw That's because tests coverage decreased with the change, you can check the file list above to find which parts missing coverage. I'm not sure if it's a blocker for merge now but we do not care much about that before :P. @freesky-edward can you please help to confirm?

Copy link
Contributor

@freesky-edward freesky-edward left a comment

Choose a reason for hiding this comment

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

thanks for your contributing, regarding to the coverage rate, it should pass before merging it, but I am not concerned this much. It can go if all is good enough.
I have some confusion about the relationship of TE, template, environment. it looks TE is an abstract struct of other two. if so, I would suggest to separate it from utility file(util.go).


// Fetch fetches the contents of a TE from its URL. Once a TE structure has a
// URL, call the fetch method to fetch the contents.
func (t *TE) Fetch() error {
Copy link
Contributor

Choose a reason for hiding this comment

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

if this is an internal method, I would prefer to name it starting with lowercase letter.

@Savasw
Copy link
Contributor

Savasw commented Jul 13, 2018

@freesky-edward As discussed, for RTS Stack, we have followed code architecture from openstack gophercloud where TE is abstract for both environment and template as there are lot of common features that work for both. Also Fetch function is not used externally at the moment but could be useful for end users separately to fetch content from URL.

@freesky-edward
Copy link
Contributor

@Savasw thanks for your response, LGTM

@freesky-edward freesky-edward merged commit 7dded5c into huaweicloud:master Jul 13, 2018
@simrannagra simrannagra deleted the rts-dev branch August 6, 2018 06:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants