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

Changes to README, instructions for adding as a dependency and using … #100

Closed
wants to merge 2 commits into from
Closed

Conversation

nikunjy
Copy link

@nikunjy nikunjy commented Aug 27, 2020

This change will not impact any kind of compatibility issues. Nothing to declare here.

README.md Outdated
```

Please note that this package at the time of this writing doesn't use v5 golang client.
You might have to also do this
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still unclear on why a separate go get would be required for something that is a transitive dependency of the package you already imported. Also not sure of the relevance of saying that it doesn't use the v5 SDK; an app that imports Relay would not normally have any interaction with Relay's use of the SDK.

Copy link
Author

Choose a reason for hiding this comment

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

Do you think adding this information will be confusing ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, I am confused... so I'd like to have a better understanding of what the importance of this information is in your opinion, in case I'm missing something. I realize I didn't put question marks on those two sentences, but I meant them as questions.

Copy link
Contributor

Choose a reason for hiding this comment

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

The part about the correct gopkg.in import path for the package is of course important— I wasn't questioning that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I should ask more specifically: did you find that if you did not do a separate go get for the SDK package, something did not work? If so, what?

Copy link
Author

Choose a reason for hiding this comment

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

In my case it didn't work out of the box without doing the go get. Maybe it is because I was using the launchdarkly client already.

Copy link
Contributor

@eli-darkly eli-darkly Sep 24, 2020

Choose a reason for hiding this comment

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

Well, when I asked "if so, what [did not work]?", I was hoping to get a little more information than "it didn't work". Like, what kind of errors did you see? Again, I'm reluctant to add advice to this readme that I wouldn't be able to explain the reason for to anyone— and if this really is a requirement under some circumstances, knowing more specifically what you saw could be helpful to us in providing support to other developers.

Copy link
Author

Choose a reason for hiding this comment

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

Removed the client info. I don't remember the error I had, it was something to do with gomod getting confused between v4 and v5 because I was already using the client.

Copy link
Contributor

Choose a reason for hiding this comment

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

it was something to do with gomod getting confused between v4 and v5 because I was already using the client

I suspect that this may have meant you were using an incorrect import path somewhere. As far as Go is concerned, gopkg.in/launchdarkly/go-server-sdk.v4 and gopkg.in/launchdarkly/go-server-sdk.v5 are two completely different and unrelated packages.

@eli-darkly
Copy link
Contributor

I'm going to reject this PR because at this point it consists only of adding an import path to the sample code in the readme— which I will do on the v5 branch in case we do any more maintenance releases of v5, and it has already been done in the revised docs for the upcoming v6 release— and adding a statement that tells people to use go get, which is not necessarily correct advice depending on what package manager they are using, but is also just repeating very basic Go programming concepts (if you import a package, you will need some way to make the package available, there is nothing special about the Relay code in that regard).

@eli-darkly eli-darkly closed this Sep 24, 2020
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.

2 participants