Skip to content

Add Module level docs #6

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

Merged
merged 1 commit into from
Aug 17, 2023
Merged

Add Module level docs #6

merged 1 commit into from
Aug 17, 2023

Conversation

G8XSU
Copy link
Collaborator

@G8XSU G8XSU commented Jul 20, 2023

Note that crate level doc is mostly a placeholder, more details regarding usage to be documented pre-release.

@G8XSU G8XSU requested review from tnull and jkczyz July 20, 2023 19:00
@G8XSU G8XSU marked this pull request as ready for review July 20, 2023 21:23
Copy link
Contributor

@tnull tnull left a comment

Choose a reason for hiding this comment

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

Even if we don't want to add a full usage example now, it would be great to spend some words on how we imagine this crate to be used and/or to give a bit more context.

E.g.: What is VSS? Where can we learn more?

src/lib.rs Outdated
use crate::client::VssClient;
use crate::error::VssError;

/// Implements a thin-client [[`VssClient`]] to access a hosted instance of Versioned Storage Service (VSS).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Implements a thin-client [[`VssClient`]] to access a hosted instance of Versioned Storage Service (VSS).
/// Implements a thin-client [`VssClient`] to access a hosted instance of Versioned Storage Service (VSS).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wanted those brackets to be there, otherwise the sentence seems a bit confusing.
"Implements a thin-client VssClient to access a hosted instance of Versioned Storage Service (VSS)."

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, but for that you might want to use parentheses rather than brackets?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i am baffled that we have to discuss this.
But i think use of brackets is correct here.

  • Brackets are used to enclose additional information or comments within a sentence. They are often used for clarification, to add explanations, or to provide context to a particular word or phrase.

in this case, it is providing additional information in the context of the thin-client implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, nevermind. Just looks a bit odd in the rendered docs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

idk, brackets don't look any different in rendered docs than parenthesis. both remain the same.

Copy link

Choose a reason for hiding this comment

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

Brackets are definitely not normally used like this and honestly I thought this was a copy-paste and/or text editor mistake. Where does your reference come from?

Most English writing style guides limit brackets to very specific uses (e.g., https://apastyle.apa.org/learn/faqs/use-brackets). I'd rather use parentheses instead for consistency with these guidelines and all our other docs.

Copy link
Collaborator Author

@G8XSU G8XSU Jul 24, 2023

Choose a reason for hiding this comment

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

Not that it has to be a source of truth but I repeatedly asked chatgpt for brackets vs parenthesis for this documentation.
Here's what it had to say:

In some cases, parentheses can also be used to enclose additional information within a sentence. However, in the context of your original sentence, using square brackets [ ] is more appropriate and common for several reasons:

  • Clarity: Square brackets are commonly used to add clarifications, annotations, or meta-information within a sentence. They stand out more and make it clear that the enclosed text is an additional comment or detail.

  • Technical Context: In many technical contexts, such as programming or documentation, square brackets are often used to indicate optional or placeholder elements, as in [VssClient]. This is a familiar convention in these fields.

While it's not necessarily incorrect to use parentheses in this context, the convention of using square brackets [ ] for annotations or optional elements is more common, especially in technical writing or when providing additional information about a specific term.

But my overarching point is that neither of them is strictly right or wrong in this context. I guess chatgpt seems to agree.

Copy link

@jkczyz jkczyz Jul 24, 2023

Choose a reason for hiding this comment

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

The first point seems to mostly agree with the style guide that I cited other than the the vague "add clarification". Square brackets are used for very specific things as noted there.

The second point doesn't agree with your use. It's talking about optional parameters to commands where the convention is to use square brackets to indicate the optionality. VssClient doesn't fit that description.

Why don't we just avoid the issue entirely by using a normal link?

Implements a [thin-client](VssClient) to access a hosted instance of Versioned Storage Service (VSS).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Replaced with '()'

@G8XSU G8XSU merged commit 3c2de18 into lightningdevkit:main Aug 17, 2023
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