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.

Sign up for GitHub

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

Jump to bottom

Fix yang case when two containers with same name , but different name… #640

Merged
4 commits merged into from Nov 30, 2017
Merged

Fix yang case when two containers with same name , but different name… #640

4 commits merged into from Nov 30, 2017

Conversation

ghost
Copy link

@ghost ghost commented Nov 27, 2017

…spaces, augment one container

Fix yang case when two containers with same name , but different name…
b4d6287 
…spaces, augment one container

 * Fix golang naming to be consistent with other langs
  * Updated samples and tests
 * address #535
 * Add more debug msgs and throw exceptions to avoid segfaults
 * tcp server start/stop
@codecov
Copy link

codecov bot commented Nov 27, 2017 •
edited
Loading

Codecov Report

Merging #640 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #640   +/-   ##
=======================================
  Coverage   32.77%   32.77%           
=======================================
  Files          31       31           
  Lines        5450     5450           
  Branches      550      550           
=======================================
  Hits         1786     1786           
  Misses       3664     3664
Impacted Files Coverage Δ
sdk/cpp/core/src/path_api.hpp 100% <ø> (ø) ⬆️

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 c787803...e2c88e7. Read the comment docs.

ylil93
ylil93 reviewed Nov 29, 2017
View reviewed changes
sdk/cpp/tests/test_netconf_provider.cpp
@@ -38,7 +38,7 @@ TEST_CASE("CreateNoRepoP")
CHECK_NOTHROW(provider.get_encoding());
}

TEST_CASE("CreateNoRepoPTCP")
Copy link
Contributor

Choose a reason for hiding this comment

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

Leave comment about running this test case

Copy link
Author

Choose a reason for hiding this comment

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

Done

sdk/cpp/tests/test_netconf_tcp_client.cpp
@@ -27,7 +27,7 @@ using namespace ydk;
using namespace std;
#define NC_VERB_VERBOSE 2

TEST_CASE("tcp_xr")
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above: leave a comment about running these test cases

Copy link
Author

Choose a reason for hiding this comment

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

Done

sdk/go/core/samples/README.md
@@ -0,0 +1,36 @@
## Running Go Samples
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for creating this read me

sdk/go/core/ydk/path/path.go
@@ -62,6 +62,7 @@ func ExecuteRPC(
panicOnCStateError(cstate)

if rootSchema == nil {
ydk.YLogError("root schema is nil!")
Copy link
Contributor

Choose a reason for hiding this comment

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

These new lines are using spaces, but the go files seem to be using tabs.

I believe python style guides recommend spaces and go style guides recommend tabs... Maybe it might be good to commit to one style for the whole project at some point in the future, but for now and for this commit, I think it's better to not mix them up.

Copy link
Author

Choose a reason for hiding this comment

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

Done

ydkgen/printer/go/module_printer.py Outdated
@@ -85,7 +85,7 @@ def _print_imports(self, package):
def _print_init(self, package):
self.ctx.writeln('func init() {')
self.ctx.lvl_inc()
self.ctx.writeln('fmt.Println("Registering top level entities for package {}")'.format(package.name))
#self.ctx.writeln('fmt.Println("Registering top level entities for package {}")'.format(package.name))
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to delete this line?

ydkgen/printer/python/class_inits_printer.py Outdated
@@ -240,3 +240,10 @@ def _print_class_setattr_body(self, clazz, leafs):
def _print_class_setattr_trailer(self):
self.ctx.lvl_dec()
self.ctx.bline()


def _get_qualified_yang_name(clazz):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you forgot to delete this

ydkgen/printer/go/class_get_children_printer.py Outdated
@@ -36,8 +36,13 @@ def print_function_body(self):
if child.is_many:
self._print_many(child)
else:
path = ''
Copy link
Contributor

Choose a reason for hiding this comment

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

leave a comment on this code

ylil93
ylil93 suggested changes Nov 29, 2017
View reviewed changes
Copy link
Contributor

@ylil93 ylil93 left a comment

Choose a reason for hiding this comment

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

see comments

@ghost ghost merged commit 457e0a1 into CiscoDevNet:master Nov 30, 2017
@ghost ghost deleted the fix_xe_native_issue branch November 30, 2017 18:23
@ghost ghost mentioned this pull request Nov 30, 2017
Closed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
1 participant
@ylil93