Add NodeBuilder for node instantiation

[Soya-Onishi]

Close #157

This PR add builder only for replacing current Node constructor, and still not implement node options for #142.
Current implmentation still remain node constructor.
However, after builder implementation, does normal constructor have demand?
If no, I'll remove constructor, and I also fix some docs and implementations.

[Soya-Onishi]

As a milestone, current NodeBuilder supports only namespace and no node options are supported.
After this PR is merged, I'll work on node options support.

This PR also has current implementation change.
Methods related to Node instantiation like Context::create_node and Node::new only delegate responsivility for instantiation of Node to NodeBuilder

[nnmm]

@Soya-Onishi there seems to have been a mixup with the recently merged PRs, could you rebase it cleanly onto master?

[nnmm]

Thanks for the PR! I left some initial remarks.

Also, feel free to explicitly request a review on your PRs when they're ready, otherwise it's easy for us to miss them.

[Soya-Onishi]

Sorry for lack of knowledge about git. Is this form desired commits which @nnmm mentioned?

[nnmm]

Functionality-wise, this looks good to me. I'd only delete the Node::new_with_namespace() and Context::create_node_with_namespace() functions.

However, I think we should aim for making the documentation excellent as well.

• The "Naming" section of the Node::new_with_namespace() function's docs should probably be moved to the Node struct
• The NodeBuilder::namespace() function should reference the naming rules for namespaces, and mention that validation only happens later, in the build() function.
• The NodeBuilder::new() function should do the same for the node name, since it takes the node name as an argument
• Every function should have an example/doctest. The example from Node::new_with_namespace() can be reused for these.
• Convenience functions should reference the original functions they're equivalent to (in this case, that's only Context::create_node_builder(), which I've left a suggestion for)

[esteve]

@Soya-Onishi thank you so much for your contribution! 🙂

[nnmm]

@Soya-Onishi Seems like there are again some accidental changes from resolving conflicts with master.

[Soya-Onishi]

From some searches, I cannot found the way to rebase with clean commit logs, For now, merge is clearner than rebase in this case(conflict appear after creating PR), I think.

[nnmm]

@Soya-Onishi That's fine, the commit log doesn't need to be clean since we usually squash when merging.

But if you're interested, I usually simply do "git rebase master" when there are conflicts with master, and if I want to squash things locally, I do "git reset --soft master && git commit" on the branch.

[nnmm]

Also, thanks for fixing the error type in the doctests!

[nnmm]

@Soya-Onishi Would you be okay with me pushing some documentation improvements directly to this branch (no code changes)? That would be easier than commenting here.

[Soya-Onishi]

@nnmm Sure, please push your commits.

[nnmm]

@Soya-Onishi Just an update, I didn't manage to do the doc improvements yet, but probably tomorrow.

[nnmm]

@Soya-Onishi Done.

From my side, this is now almost good to go. I have three suggestions:

• Could you create a new file for the node builder (node/builder.rs)? It might require marking a few fields in node/mod.rs as pub(super), but that's okay.
• Could you swap the arguments to NodeBuilder::new()?
• We could have a Node::builder(context: &Context, node_name: &str) -> NodeBuilder method instead of NodeBuilder::new(). That is a common pattern because it's easier to discover when looking at the Node struct, and it's also slightly shorter,

[Soya-Onishi]

@nnmm Should I swap arguments of Node::new() also?

[nnmm]

@nnmm Should I swap arguments of Node::new() also?

Yes, please.

[nnmm]

@esteve Could you maybe re-review and see if you feel like you can remove the changes requested status? Then I'd do a final review of documentation.

[nnmm]

Only four small improvements, then this is good to go from my side!

[nnmm]

Looks good to me!

@esteve ptal!

[esteve]

@Soya-Onishi thanks for iterating and addressing our feedback!

[Soya-Onishi]

Thanks for your suggestions and cooperation!!