search

IBM / cloudant-go-sdk

Cloudant SDK for Go
Apache License 2.0
21 stars 12 forks source link

Change var name from `examplesClient` to `client` in `get_info_from_existing_database.go` example #35

Closed eiri closed 3 years ago

eiri commented 3 years ago

Describe the bug All examples are using client for an instance of cloudantv1.Service, but get_info_from_existing_database is using examplesClient.

Expected behavior Change var name to client for docs consistency

mojito317 commented 3 years ago

The reason for using a different name for the example in get_info_from_existing_database is because that client must be different to read a real and existing database.

While in the first example the client connects to the https://examples.cloudant.com/ service URL, in the other cases the client connects to a user-owned URL.

eiri commented 3 years ago

What do you mean "must"? It can be a different client, it can be the same client switching to a different service programmatically, there is no hard requirement for having a client instance per service.

This is orthogonal here though, because there is no other clients in this example, all the examples are separate files with own main block, so they are different programs and each one is initiating independent client, so there is no reason to use a different variable name in just one of them.

mojito317 commented 3 years ago

What do you mean "must"? It can be different client, it can be the same client switching to the different service programmatically, there is no hard requirement on having an client instance per service.

We wanted to show an easy example here, and not make things more difficult e.g. by switching to different services programmatically. examplesClient shows us it is a different client than the client. It was all about to stick to the easiness. The animaldb can be read by any user, maybe it encourages people to use this library if they see first an example that does not need backend settings.

This is orthogonal here though, because there is no other client in that one example, all the examples are separate files with own main block, so they are different programms and each one is initiating independent client, so there is no reason to use a different variable name in just one of them.

You are right. There are some missing pieces I’ve forgotten about. When this was written we agreed that the code snippets should be copy-pasted under each other without any problem by the user, that is why I created a separate client for the examples host. Then it turned out we should show the imports in a separate block of code. What I missed is I introduced the embedmd and due to the formatting I did not want to separate the code anymore for imports and the examples in a separate code block, because I did wanted to upload a working code and regardless embedmd can be used to have a special range of code lines in the markdown, but it cannot handle the indentations.

Another thing, if we want to have the client then it should be corrected in all of the sdk repos.

eiri commented 3 years ago

Right, I see the historic reasons for the difference, but is it still valid? I don't see where embedmd used to create the README, as for examples, as they are right now, they are not valid to be copy-pasted under each other without editing, e.g. there is a reassignment of document variable in update and delete docs files.

Another thing, if we want to have the client then it should be corrected in all of the sdk repos.

True, but this is outside of scope of this issue.

mojito317 commented 3 years ago

is it still valid

No, it is not. I agree now with your modification.

True, but this is outside of scope of this issue.

It wanted to be just a warning.