Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Golang services #605

Merged
10 commits merged into from
Oct 18, 2017
Merged

Golang services #605

10 commits merged into from
Oct 18, 2017

Conversation

ylil93
Copy link
Contributor

@ylil93 ylil93 commented Oct 18, 2017

  • netconf services, tests, doc
  • executor services, tests, doc
  • logging wrapper
  • fixed bugs in api gen
  • fixed bugs in testing

Copy link

@ghost ghost 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 this change, Lily! Looks good. Just one minor comment.

@@ -406,17 +508,17 @@ func getDataNodeFromEntity(state *types.State, entity types.Entity, rootSchema C
func walkChildren(state *types.State, entity types.Entity, dataNode C.DataNode) {
children := entity.GetChildren()

fmt.Printf("Got %d entity children\n", len(children))
ydk.YLogInfo(fmt.Sprintf("Got %d entity children", len(children)))
Copy link

Choose a reason for hiding this comment

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

These should be YLogDebug instead of YLogInfo. Info should only be used for end-user relevant logs. These messages are more relevant for ydk developers, not end user.


for childName := range children {

fmt.Printf("Lookin at entity child '%s'\n", children[childName].GetSegmentPath())
ydk.YLogInfo(fmt.Sprintf("Looking at entity child '%s'", children[childName].GetSegmentPath()))
Copy link

Choose a reason for hiding this comment

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

Same as above. These should be YLogDebug instead of YLogInfo. Info should only be used for end-user relevant logs. These messages are more relevant for ydk developers, not end user.

@ylil93
Copy link
Contributor Author

ylil93 commented Oct 18, 2017

made the suggested changes

@codecov
Copy link

codecov bot commented Oct 18, 2017

Codecov Report

Merging #605 into golang will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@          Coverage Diff           @@
##           golang    #605   +/-   ##
======================================
  Coverage    73.8%   73.8%           
======================================
  Files         104     104           
  Lines       10271   10271           
  Branches     1359    1359           
======================================
  Hits         7581    7581           
  Misses       2431    2431           
  Partials      259     259
Impacted Files Coverage Δ
ydkgen/printer/go/class_constructor_printer.py 14.1% <0%> (ø) ⬆️
ydkgen/printer/go/class_printer.py 26.77% <0%> (ø) ⬆️
ydkgen/printer/go/class_has_data_printer.py 22.85% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 66af9a6...bd8994b. Read the comment docs.

@ghost ghost merged commit 2258e2d into CiscoDevNet:golang Oct 18, 2017
@ylil93 ylil93 deleted the golang_services branch November 1, 2017 22:08
This pull request was closed.
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.

1 participant