-
Notifications
You must be signed in to change notification settings - Fork 43
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
docs: update onboarding steps for Go v3 client #6987
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From my point of view, this looks good, and along with the new API for the client. Great job! 👍
I've decided to go ahead with changes only to GO and will update Java onboarding in another branch. I'd rather we get this part of the UI updated ASAP instead of possibly having it held until January because of 1) Christmas Holidays 2) Unexpected obstacles in updating onboarding for Java. BTW. Not sure what to do about the
|
Hi @karel-rehor, sorry for the delay on this - I was working on fixing the CI pipeline and I see it has passed the tests now. I'll take a look at this PR in more detail tomorrow. Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello! Thanks for doing this! I apologize for not getting to this when I said I would, it slipped my mind completely. Similar to the Java onboarding, it would be helpful for a video demonstrating the changes since I noticed some spacing and formatting changes, but it looks good to me otherwise! If the video size limit is an issue here, let me know and I can run the branch locally to see these changes
@@ -48,7 +48,8 @@ export const WriteDataComponent = (props: OwnProps) => { | |||
onSelectBucket(bucket.name) | |||
}, [bucket, onSelectBucket]) | |||
|
|||
const codeSnippet = `org := "${org.name}" | |||
const codeSnippet = `// FOO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the // FOO
line is the only thing changed here, is that for consistency?
"count": 40, | ||
}, | ||
} | ||
const writeCodeSnippet = ` data := map[string]map[string]interface{}{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious to see what effect this whitespace has on the snippet
panic(err) | ||
} | ||
if err != nil { | ||
panic(err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also curious about the whitespace here, looks like the number of spaces in a tab has changed, is that maintained across all snippets in the go onboarding?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @karel-rehor - thanks for doing this. This looks good based on my review, as long as the rationale for doing this is the same as in the Go PR: i.e., this is just to update to conform to changes to the influxdb v3 client libraries.
Before we approve this to merge in, could you please add a video confirming we don't end up with any visual artifacts here? Thanks very much.
Updates the onboarding steps for Go v3 client
Checklist
Authors and Reviewer(s), please verify the following: