Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: Provides 'Adding a Visual Studio Code Extension' module #2265
feat: Provides 'Adding a Visual Studio Code Extension' module #2265
Changes from 3 commits
17b9e1b
0791149
fccfa05
8df50e1
78cd110
5cdb312
222c80a
7aa844d
39a6597
cca238d
4a9934f
b1eaf41
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
In some che-docs topics,
link:
precedes URLs like this, but not in this PR. Is that okay?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.
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'm having trouble making sense of the final sentence.
airGapContainerRegistryHostname
?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.
Using the definite article, "the default plug-ins", makes more sense if the "registry project" topic describes specific plug-ins. Otherwise, if it's an abstract discussion of "plug-ins, extensions, and source code," the current wording is fine.
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.
'air gap' is also named 'restricted environment' sometimes. But you can find definition there https://en.wikipedia.org/wiki/Air_gap_(networking)
And it's used in Che (and docs)
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.
Although we should strive to use the terminology consistently, I'm not highlighting "air gap" as a problem. I'm concerned that calling it a "mode" might prompt users to search for an actual software mode when it seems like there might not be one. I'm guessing you mean "an air-gapped or restricted environment" instead.
Otherwise, if there is something like a mode, it's not called a "mode" in the software or documentation and actually consists of one or more parameters that the user must set when operating the software in a restricted environment.
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'll accept any suggested changes but here suggested changes have questions so I never saw this kind of suggested changes.
Does "only the default registry is available there" mean "only the default registry is available in an air-gapped environment?" This seems like the inverse of what I would expect: That the user probably requires use of a custom plug-in or extension registry because they are in an air-gapped environment. The default URLs of the plug-ins and extensions might not be reachable from an air-gapped system.
There is no need to use a custom plug-in or extension registry. Che deploy a registry on the air-gapped environment and in that case it includes all the binaries so there is no need to fetch something externally.
everything is reachable in that case (we build and deploy an offline / air-gapped environment registry)
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.
wording fixed
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.
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.
GitHub suggestion implies that we can commit directly the change.
Here it looks that there are two suggestions so it should be two separate GitHub PR suggestions (and people can approve one of them)
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.
wording fixed
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.
Didn't find any other instances of
*EXAMPLE*
in che-docs. Consider adopting the standard code block titles throughout, like this: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.
formatting fixed
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.
For comparison, I searched the che-docs repo for admonitions like one this but didn't find any. You can use the asciidoc formatting for admonitions, which looks like this:
or
The openshift-docs repo standardized on this later format...
I would recommend asking your team which one to use.
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.
This is how asciidoc renders the original format and the two proposed formats:

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.
formatting fixed
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.
s/id/ID/ throughout.
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.
fixed
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.
Bulleted lists don't render as such unless they're preceded by a blank line.
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.
formatting fixed
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.
add...to
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.
wording fixed
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.
"add...to" here and elsewhere
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.
wording fixed
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.
For minimalism:
Is there a more commonly understood equivalent for "inlining" that preserves the meaning?
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.
wording fixed
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.
Part of the issue with using the adjective, "inline," as a verb is that when you use it next to a noun, as shown here, the reader might experience some semantic noise:
raising the question, "Is a verb missing between "to" and "inline extension attributes?"
Perhaps subsitute it with "add" or "insert?" WDYT?
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.
reworded as "inlining the extension/plug-in attributes"