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

remove self nodelocal examples #17979

Merged
merged 2 commits into from
Nov 10, 2023
Merged

remove self nodelocal examples #17979

merged 2 commits into from
Nov 10, 2023

Conversation

dt
Copy link
Member

@dt dt commented Oct 9, 2023

We do not recommend self in most cases and showing it as an example gives it more exposure than we really desire.

We especially want to be sure that anyone using "self" has read and understood the footnote under "considerations". This note was previously linked after the example but that relied on one clicking it and reading it, so a user could presumably discover and start using self without reading the note.

After this change, the only mention of self is in the footnote, so only users who read the note should be aware it is an option, specifically in the configuration where it is a valid option.

@dt dt requested review from benbardin and kathancox October 9, 2023 14:46
@github-actions
Copy link

github-actions bot commented Oct 9, 2023

@netlify
Copy link

netlify bot commented Oct 9, 2023

Deploy Preview for cockroachdb-interactivetutorials-docs canceled.

Name Link
🔨 Latest commit 9608e09
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-interactivetutorials-docs/deploys/654e8401cb28be00084bf4a9

@netlify
Copy link

netlify bot commented Oct 9, 2023

Deploy Preview for cockroachdb-api-docs canceled.

Name Link
🔨 Latest commit 9608e09
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-api-docs/deploys/654e8401a7d5c90008939d1e

@netlify
Copy link

netlify bot commented Oct 9, 2023

Netlify Preview

Name Link
🔨 Latest commit 9608e09
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-docs/deploys/654e84013e27a800075f1dc9
😎 Deploy Preview https://deploy-preview-17979--cockroachdb-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@kathancox kathancox left a comment

Choose a reason for hiding this comment

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

LGTM, as per the PR description the only mention of self remains in the footnote to the main table on this page.

@benbardin
Copy link
Contributor

I agree with taking this out of the examples, but I worry the footnote is still too hard to find, and in particular the note about "common bucket" not super-discoverable. What do you two think of that? Is it fine? Or is there a way we can make it a teeny bit more prominent?

@kathancox
Copy link
Contributor

kathancox commented Oct 9, 2023

I agree with taking this out of the examples, but I worry the footnote is still too hard to find, and in particular the note about "common bucket" not super-discoverable. What do you two think of that? Is it fine? Or is there a way we can make it a teeny bit more prominent?

Depending on how we feel about customers using Nodelocal, (I don't have a good understanding of this), we could just put the footnote into the Parameters cell of the table for nodelocal. This is the only place we really discuss nodelocal and it seems a little annoying to have this buried in the footnote as you say. (The content is about the parameters for the URL after all.)

David, let me know if you want me make any changes.

@dt
Copy link
Member Author

dt commented Oct 9, 2023

I worry the footnote is still too hard to find

That seems fine, to me, as long as finding it is the only way you'd ever discover self, right?

We want -- nay, need -- you to find the footnote and read it if you're going to use self. So making it easier to find, or indeed impossible not to find, would be important if there were other ways to find self. But if the footnote is the only way to find self exists, then I don't know if we need to make it easier to find or more prominent, and indeed doing so would give more exposure_ to self if the footnote is the only place we acknowledge the existence of self.

@benbardin
Copy link
Contributor

doing so would give more exposure to self
Yes, I agree this is the implication. My question is about the judgment call - do we want to make nodelocal://self hard to find? I tend to think no. It's a supported feature with real use cases. So given that, I tend to think we should make the feature and its correct usage a bit easier to find than exclusively in a footnote. You think otherwise, though?

@dt
Copy link
Member Author

dt commented Oct 9, 2023

Correct, I think self should be harder to find that normal (explicit ID in URI) nodelocal, which is what is left by itself on the example line after this change. Explicit ID nodelocal is what we think most people who use local storage should be using, while self is only intended to be used in a very narrow special-case. Since we only want people in that very narrow special case to ever consider using self, I think it is desirable that to find it and potentially (mis)use it, you need to go read the footnote and go "yes, I am in the this specific special-case that this note is addressing".

Copy link
Contributor

@benbardin benbardin left a comment

Choose a reason for hiding this comment

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

OK, I like it!

@kathancox
Copy link
Contributor

@dt do you want to remove these examples from v23.2 docs as well?

dt and others added 2 commits November 10, 2023 14:26
We do not recommend `self` in most cases and showing it as an example gives it more exposure than we really desire.

We especially want to be sure that anyone using "self" has read and understood the footnote under "considerations". This note was previously linked after the example but that relied on one clicking it and reading it, so a user could presumably discover and start using `self` without reading the note.

After this change, the only mention of `self` is in the footnote, so only users who read the note should be aware it is an option, specifically in the configuration where it is a valid option.
@kathancox kathancox merged commit 0847e18 into main Nov 10, 2023
3 checks passed
@kathancox kathancox deleted the dt-nodelocal-self branch November 10, 2023 19:31
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.

3 participants