chore/feat: enforce short lines in for-developers#42
chore/feat: enforce short lines in for-developers#42arikorn wants to merge 15 commits intobitfocus:mainfrom
Conversation
This prevent word-wrap, which often ends up looking odd in the browser ("paragraphs" without proper separation).
[Docusaurus recommends](https://docusaurus.io/docs/markdown-features/admonitions#usage-with-prettier) blank-lines after the opener (`:::note`) and before the closer (`:::`), in part, because prettier word-wraps these incorrectly...
run ``` yarn prettier --prose-wrap always --print-width 80 --parser mdx --write "**/*.md" ```
Bonus: (not directly needed for this PR) - Replace `![image]()` with a descriptive label.
This improves readability and simplifies some cases. (A complete search of the repo did not reveal any cases in which this breaks the current text-flow, i.e. by breaking up a single line.) Note a nice side-effect is that the image will always appear on its own line. (If you need to override this, it should be possible with inline html/style.) Note: this is prelude to an internal reformat of for-developers (i.e. reformat the files with essentially no change to the built website)
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a Prettier override for developer-facing Markdown and applies widespread formatting reflow, line-wrapping, minor wording tweaks, a small git config command addition, and one OAuth callback example; no API signatures or executable logic were changed. Changes
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (17)
for-developers/git-workflows/github-workflow.md-83-84 (1)
83-84:⚠️ Potential issue | 🟡 MinorMinor wording typo in remote-repository sentence
Small grammar fix for clarity: “need two ‘know’ two remote repos” reads like an accidental edit artifact.
Proposed fix
-Your local repository will need two "know" two remote (GitHub) repos -- one for +Your local repository will need to know two remote (GitHub) repos -- one forfor-developers/git-workflows/github-workflow.md-119-120 (1)
119-120:⚠️ Potential issue | 🟡 MinorTypo: “feature of bug-fix” should be “feature or bug-fix”
Tiny copy edit, but helpful for readability.
Proposed fix
-Next, modify/add code for your feature of bug-fix using your favorite coding +Next, modify/add code for your feature or bug-fix using your favorite codingfor-developers/maintenance-team/making-a-release-of-companion.md-28-30 (1)
28-30:⚠️ Potential issue | 🟡 MinorFriendly suggestion: Capitalize "GitHub"
Since you're already updating these lines, it would be great to capitalize "GitHub" to match the official branding. This appears in both the major/minor release section (line 28) and the patch release section (line 55).
✨ Proposed fix
For lines 28-30:
-- Create the new github release https://github.com/bitfocus/companion/releases +- Create the new GitHub release https://github.com/bitfocus/companion/releases using the previous one as a template, replacing the changes with the contents of the changelogFor lines 55-57:
-- Create the new github release https://github.com/bitfocus/companion/releases +- Create the new GitHub release https://github.com/bitfocus/companion/releases using the previous one as a template, replacing the changes with the contents of the changelogAlso applies to: 55-57
for-developers/maintenance-team/making-a-release-of-companion.md-42-43 (1)
42-43:⚠️ Potential issue | 🟡 MinorFriendly suggestion: Add apostrophe to "doesn't"
Just a small typo fix: "doesnt" should be "doesn't" in both places where you're running the release and checking the top bar.
✨ Proposed fix
For lines 42-43:
-- Run the release, and make sure the top bar doesnt report the build as +- Run the release, and make sure the top bar doesn't report the build as experimental or out of dateFor lines 64-65:
-- Run the release, and make sure the top bar doesnt report the build as +- Run the release, and make sure the top bar doesn't report the build as experimental or out of dateAlso applies to: 64-65
for-developers/module-development/module-lifecycle/upgrading-a-module-built-for-companion-2.x.md-771-771 (1)
771-771:⚠️ Potential issue | 🟡 MinorTypo: "any by" should be "and by"
There's a small typo in this line. "any by combining" should be "and by combining".
✏️ Proposed fix
-surprisingly slow on some OSes, any by combining it all into a few files, we can +surprisingly slow on some OSes, and by combining it all into a few files, we canfor-developers/module-development/home.md-68-69 (1)
68-69:⚠️ Potential issue | 🟡 MinorTypo: "for you device" should be "for your device"
The AI summary indicates this was corrected, but it looks like the typo is still present in the code. This should read "for your device" instead of "for you device".
📝 Suggested fix
-With over 700 published modules, you may find a module already written for you -device. You can find the GitHub repository for each module by searching +With over 700 published modules, you may find a module already written for your +device. You can find the GitHub repository for each module by searchingfor-developers/module-development/local-modules.md-19-24 (1)
19-24:⚠️ Potential issue | 🟡 MinorFix capitalization and punctuation in the numbered list.
A few small content issues in this section:
- Line 19: "github" should be capitalized as "GitHub"
- Line 19: Missing punctuation after "github" – the text runs two sentences together
- Line 24: The sentence feels incomplete – "to be loaded" by what?
Thanks for the formatting improvements overall! These are just small refinements that would polish the documentation.
📝 Suggested fixes
-1. A git clone of a module from github This requires some additional setup, as +1. A git clone of a module from GitHub. This requires some additional setup, as the module will need to be prepared with a `yarn install`, and for some, a `yarn build`. 2. Packaged output This is a folder that contains a `companion/manifest.json`, `companion/HELP.md`, `package.json`, `main.js` (or another name), and - possibly a few other files. No extra work is needed for this to be loaded + possibly a few other files. No extra work is needed for this to be loaded.for-developers/Satellite-API.md-40-41 (1)
40-41:⚠️ Potential issue | 🟡 MinorTighten protocol wording to avoid implementer confusion.
A couple of typos in these lines make the spec harder to read:
- Line 40: “can as both”
- Line 49: “followers” should be “follows”
✍️ Suggested wording cleanup
-Note: You can send boolean values can as both true/false and 0/1, you will +Note: You can send boolean values as both true/false and 0/1; you will always receive them as 0/1 @@ -field to check compatibility with companion. The number followers +field to check compatibility with companion. The number follows [semver](https://semver.org/) for versioning. We hope to keep breaking changesAlso applies to: 48-50
for-developers/core-development/development-flow.md-46-49 (1)
46-49:⚠️ Potential issue | 🟡 MinorGreat reflow overall — a few quick typo touch-ups would help readability.
I noticed a few small wording/typo issues in changed lines:
- Line 48:
subdirectoy→subdirectory- Line 96: “Keep the up-to-date…” appears to be missing a noun
- Line 158:
open a a second terminal📝 Suggested edits
-be added as a subdirectoy) then: +be added as a subdirectory) then: @@ -Keep the up-to-date by fetching (`git fetch`) or pulling (`git pull`). +Keep your repo up to date by fetching (`git fetch`) or pulling (`git pull`). @@ -itself in development mode open a a second terminal window/tab and run +itself in development mode, open a second terminal window/tab and runAlso applies to: 96-99, 155-159
for-developers/module-development/connection-basics/user-configuration.md-24-26 (1)
24-26:⚠️ Potential issue | 🟡 MinorSmall grammar/terminology fixes in updated text.
Two quick polish items in changed prose: use “fields … are” (plural agreement), and prefer “non-JSON-safe” for consistency/readability.
Suggested edit
-The fields available for secrets is quite limited, as we expect it to only be +The fields available for secrets are quite limited, as we expect it to only be useful for API keys, usernames, passwords and similar things. If other field types would be useful, let us know and we can look at adding more.-how they are stored. Make sure to not try and use a non-json safe object or it +how they are stored. Make sure not to use a non-JSON-safe object or it either won't be saved, or will throw an error that can crash your module.Also applies to: 62-64
for-developers/module-development/module-lifecycle/companion-module-library.md-57-58 (1)
57-58:⚠️ Potential issue | 🟡 MinorTiny wording polish: use “up-to-date.”
Nice cleanup overall. One small text-quality fix: “up to date” should be hyphenated as “up-to-date” in this context.
Suggested edit
-This library is less picky about the version, and you will benefit by running an -up to date version. +This library is less picky about the version, and you will benefit by running an +up-to-date version.for-developers/setting-up-WSL.md-66-67 (1)
66-67:⚠️ Potential issue | 🟡 MinorAvoid hardcoding a single Windows host IP for WSL networking.
Great guide overall. One accuracy issue:
172.28.96.1is not stable across all WSL setups. Recommend wording this as environment-dependent and guiding users to discover the host IP dynamically.Suggested edit
-to access your host OS, you have to use 172.28.96.1. Verify this using the -command `ip route` in your Linux shell. +to access your host OS, use the Windows host IP for your current WSL network +setup (it can vary by machine/session). Verify it using `ip route` in your +Linux shell.for-developers/module-development/module-lifecycle/index.md-3-4 (1)
3-4:⚠️ Potential issue | 🟡 MinorMinor grammar tweak: use plural “tasks.”
Line 3 and Line 8 read more naturally with “tasks” instead of “task.”
Suggested patch
-description: The task necessary to release, maintain and upgrade a module repository over +description: The tasks necessary to release, maintain and upgrade a module repository over time. @@ -This section describes the task necessary to package, deliver, maintain and +This section describes the tasks necessary to package, deliver, maintain and upgrade a module repository over time.Also applies to: 8-9
for-developers/module-development/connection-basics/feedbacks.md-242-243 (1)
242-243:⚠️ Potential issue | 🟡 MinorMinor wording fixes needed for readability and correctness.
A few phrases read ungrammatically in the updated text (Line 242, Line 287, Line 311), which can confuse readers.
Suggested doc wording patch
- built-in system. It expects a parameter describe the feedbacks to process, and + built-in system. It expects a parameter describing the feedbacks to process, and - On the feedback definition, it is possible to register an additional callbacks + On the feedback definition, it is possible to register additional callbacks - options of an feedback on a button is changed, only the callback will be called + options of a feedback on a button are changed, only the callback will be called.Also applies to: 287-289, 309-312
for-developers/module-development/connection-basics/upgrade-scripts.md-113-115 (1)
113-115:⚠️ Potential issue | 🟡 MinorA couple of small wording fixes would improve clarity.
There are a few minor grammar issues in the updated lines (Line 114, Line 209, Line 211).
Suggested patch
-We recommend defining the functions in a dedicated `upgrades.ts` file, as they -should not depend on your main class and this helps avoids files growing too +We recommend defining the functions in a dedicated `upgrades.ts` file, as they +should not depend on your main class and this helps avoid files growing too long to be manageable. @@ -In your upgrade script, you are expected to return an object which describes -which actions or feedbacks changed and whether the new config object. +In your upgrade script, you are expected to return an object which describes +which actions or feedbacks changed and whether there is a new config object. @@ -Any values in this can be new or cloned objects, or a mutated in place object +Any values in this can be new or cloned objects, or a mutated in-place object from props.Also applies to: 208-212
for-developers/module-development/module-lifecycle/testing-a-custom-version-of-@companion-module-base.md-27-31 (1)
27-31:⚠️ Potential issue | 🟡 MinorSmall typo in setup instructions.
Line 27 has a typo (
havent), which is easy to fix and helps polish the step-by-step flow.Suggested patch
-1. clone this repository somewhere on your computer if you havent already +1. Clone this repository somewhere on your computer if you haven't alreadyfor-developers/module-development/module-setup/manifest.json.md-8-11 (1)
8-11:⚠️ Potential issue | 🟡 MinorIntro sentence could be tightened for readability.
Line 9 currently reads like a run-on and misses punctuation before “This provides...”.
Suggested patch
-Starting with Companion 3.0, Companion looks at the `companion/manifest.json` -file for module information (before 3.0 it looked at `package.json`) This -provides a companion specific and programming language agnostic manifest about +Starting with Companion 3.0, Companion looks at the `companion/manifest.json` +file for module information (before 3.0 it looked at `package.json`). This +provides a Companion-specific and programming-language-agnostic manifest about your module. In the future this will allow us to do more powerful things!
🧹 Nitpick comments (13)
for-developers/module-development/connection-advanced/permissions.md (1)
53-53: Optional style refinements (totally nitpicky!)The static analysis tools flagged a couple of minor style suggestions if you're interested:
- Line 53: Could be slightly more concise as "bypass all the restrictions" (dropping "of")
- Line 87: The phrase "may need to" appears in a few nearby sentences – might add a bit of variety to swap one out (e.g., "might need to" or "requires talking to")
These are completely optional and don't affect readability at all – just sharing in case you're doing any broader copyediting down the line! 😊
Also applies to: 87-87
for-developers/module-development/module-lifecycle/upgrading-a-module-built-for-companion-2.x.md (1)
647-647: Consider capitalizing "GitHub" properly"github" should be "GitHub" (with capital H) for proper capitalization of the platform name.
📝 Proposed fix
-`InstanceBase`, then do let us know on slack or github, and we will either +`InstanceBase`, then do let us know on slack or GitHub, and we will eitherfor-developers/module-development/module-setup/file-structure.md (1)
8-173: Nice work on the formatting cleanup! 📝The prose wrapping and line length consistency look great. This will definitely make diffs easier to review going forward.
If you'd like to polish things a bit further while you're here, the static analysis tools flagged a couple of minor style suggestions (though these appear to be pre-existing, not introduced by your changes):
- Line 86: "all of the listed dependencies" could be shortened to "all the listed dependencies"
- Line 124: "MIT licensed project" → "MIT-licensed project" (hyphenate compound modifiers)
- Line 151: "markdown" should be capitalized as "Markdown" (proper noun)
These are totally optional nitpicks though – the formatting work itself is solid!
for-developers/git-workflows/versioning.md (3)
15-17: Optional polish: Minor grammar tweakWhile you're polishing the formatting, you might want to adjust "which impact" to "what impact" for grammatical correctness.
✨ Suggested wording improvement
The main goal of our versioning scheme is to make sure that it is easy for the users of the different parts of Companion to estimate whether different versions -are compatible to each other and which impact a different version may have. +are compatible to each other and what impact a different version may have.
71-74: Optional polish: Simplify wordingThe phrase "by the use of" could be simplified to just "using" or "through" for more concise documentation.
✨ Suggested wording improvement
-Other applications or modules communicate with Companion by the use of +Other applications or modules communicate with Companion using application programming interfaces (APIs). It is the ultimate goal of API versioning to ensure compatibility. We are strictly following semantic versioning for our APIs.
82-86: Optional polish: Improve phrasingTwo minor improvements while you're here:
- Line 83: "not compatible to" → "not compatible with" (or "incompatible with")
- Line 86: "any more" → "anymore" (single word when used as an adverb)
✨ Suggested wording improvements
- The major-version is counted up if there are breaking changes in the module. - E.g.: the module is not compatible to the API used before (common for modules + E.g.: the module is not compatible with the API used before (common for modules upgraded for Companion v3), the module drops support for a device or a variant of a device, the module changes in a way that stored user data can't be used - any more (no upgrade script provided) + anymore (no upgrade script provided)for-developers/module-development/home.md (2)
50-50: Optional: Consider simplifying "prior to" to "before"For more concise language, you could replace "prior to" with "before": "In versions before v4.1..."
This is just a minor style suggestion and totally optional! 😊
110-110: Optional: Consider "all the" instead of "all of the"For slightly more concise language, you could use "all the places" instead of "all of the places".
Again, this is just a small style suggestion and completely optional!
for-developers/module-development/local-modules.md (2)
14-17: Optional: Consider simplifying "Inside of" to "Inside".The phrase "Inside of this folder" could be simplified to just "Inside this folder" for more concise prose. This is a minor style suggestion and totally optional! As per static analysis hint: "This phrase is redundant. Consider using 'Inside'."
34-48: Minor terminology inconsistency.On line 44, the text refers to "the developer module" (singular), but line 47 uses "developers module" (plural possessive without apostrophe). Consider using "developer module" consistently, or if possessive is intended, "developer's module".
🔧 Suggested consistency fix
- don't see the developers module, please check the log and switch on debug, + don't see the developer module, please check the log and switch on debug,for-developers/setting-up-developer-environment.md (1)
103-103: Optional: Consider using Apple's official capitalization.Apple's official branding uses "macOS" (lowercase 'm', capital 'OS') rather than "MacOS". This is just a minor style point though - feel free to keep as-is if you prefer!
📝 Suggested change
-on MacOS upgrade fnm using Homebrew: `brew upgrade fnm` (you can also install +on macOS upgrade fnm using Homebrew: `brew upgrade fnm` (you can also installfor-developers/module-development/api-changes/v2.0.md (1)
177-180: Optional: Consider hyphenating compound adjectives.The phrase "user friendly" appears twice and would be more grammatically correct as "user-friendly" when used as a compound adjective before a noun (e.g., "user-friendly values").
✏️ Suggested wording improvement
-to consider if the values used by any dropdown fields are user friendly. +to consider if the values used by any dropdown fields are user-friendly. -device, but are not at all user friendly. Some examples: +device, but are not at all user-friendly. Some examples:for-developers/module-development/api-changes/v1.12.md (1)
46-47: Optional: Minor wording clarity suggestion.The phrase "We advise all uses to be updated" reads slightly awkwardly. Consider rephrasing to something like "We advise updating all instances" or "We recommend migrating all existing code" for better flow.
This is completely optional and the current wording is understandable!
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (62)
.prettierrcfor-developers/Satellite-API.mdfor-developers/core-development/build-companion.mdfor-developers/core-development/development-flow.mdfor-developers/core-development/intro-to-core.mdfor-developers/git-workflows/git-crashcourse.mdfor-developers/git-workflows/github-workflow.mdfor-developers/git-workflows/installing-git.mdfor-developers/git-workflows/versioning.mdfor-developers/index.mdfor-developers/linking-to-companion.mdfor-developers/maintenance-team/making-a-release-of-companion.mdfor-developers/module-development/api-changes/index.mdfor-developers/module-development/api-changes/v1.10.mdfor-developers/module-development/api-changes/v1.12.mdfor-developers/module-development/api-changes/v1.13.mdfor-developers/module-development/api-changes/v1.14.mdfor-developers/module-development/api-changes/v1.5.mdfor-developers/module-development/api-changes/v1.7.mdfor-developers/module-development/api-changes/v1.8.mdfor-developers/module-development/api-changes/v2.0.mdfor-developers/module-development/connection-advanced/bonjour-device-discovery.mdfor-developers/module-development/connection-advanced/http-handler.mdfor-developers/module-development/connection-advanced/learn-action-feedback-values.mdfor-developers/module-development/connection-advanced/migrating-legacy-to-boolean-feedbacks.mdfor-developers/module-development/connection-advanced/oauth.mdfor-developers/module-development/connection-advanced/permissions.mdfor-developers/module-development/connection-advanced/setting-custom-variables.mdfor-developers/module-development/connection-basics/actions.mdfor-developers/module-development/connection-basics/connecting.mdfor-developers/module-development/connection-basics/feedbacks.mdfor-developers/module-development/connection-basics/index.mdfor-developers/module-development/connection-basics/input-field-types.mdfor-developers/module-development/connection-basics/logging.mdfor-developers/module-development/connection-basics/overview.mdfor-developers/module-development/connection-basics/presets-1.x.mdfor-developers/module-development/connection-basics/presets.mdfor-developers/module-development/connection-basics/upgrade-scripts.mdfor-developers/module-development/connection-basics/user-configuration.mdfor-developers/module-development/connection-basics/variables.mdfor-developers/module-development/home.mdfor-developers/module-development/index.mdfor-developers/module-development/local-modules.mdfor-developers/module-development/module-debugging.mdfor-developers/module-development/module-development-101.mdfor-developers/module-development/module-lifecycle/companion-module-library.mdfor-developers/module-development/module-lifecycle/index.mdfor-developers/module-development/module-lifecycle/module-packaging.mdfor-developers/module-development/module-lifecycle/releasing-your-module.mdfor-developers/module-development/module-lifecycle/renaming-your-module.mdfor-developers/module-development/module-lifecycle/testing-a-custom-version-of-@companion-module-base.mdfor-developers/module-development/module-lifecycle/updating-nodejs.mdfor-developers/module-development/module-lifecycle/upgrading-a-module-built-for-companion-2.x.mdfor-developers/module-development/module-setup/code-quality.mdfor-developers/module-development/module-setup/file-structure.mdfor-developers/module-development/module-setup/index.mdfor-developers/module-development/module-setup/manifest.json.mdfor-developers/module-development/module-setup/typescript-config.mdfor-developers/module-development/module-setup/unit-testing.mdfor-developers/setting-up-WSL.mdfor-developers/setting-up-developer-environment.mdsrc/css/custom.css
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
for-developers/module-development/module-lifecycle/testing-a-custom-version-of-@companion-module-base.md (1)
31-32: Optional wording polish: “checkout” → “check out”.At Line 31, using “check out” as a verb phrase is a bit clearer in docs.
Suggested patch
-3. If the version you want is a branch, checkout the branch, or main if you want +3. If the version you want is a branch, check out that branch, or `main` if you want
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.prettierrcfor-developers/git-workflows/github-workflow.mdfor-developers/module-development/api-changes/v2.0.mdfor-developers/module-development/module-lifecycle/index.mdfor-developers/module-development/module-lifecycle/releasing-your-module.mdfor-developers/module-development/module-lifecycle/testing-a-custom-version-of-@companion-module-base.mdfor-developers/module-development/module-setup/index.mdfor-developers/module-development/module-setup/typescript-config.mdfor-developers/module-development/module-setup/unit-testing.mdfor-developers/setting-up-WSL.mdfor-developers/setting-up-developer-environment.mdsrc/css/custom.css
✅ Files skipped from review due to trivial changes (2)
- for-developers/module-development/module-setup/index.md
- for-developers/module-development/module-setup/typescript-config.md
🚧 Files skipped from review as they are similar to previous changes (2)
- for-developers/module-development/module-lifecycle/releasing-your-module.md
- for-developers/module-development/module-lifecycle/index.md
| Your local repository will need two "know" two remote (GitHub) repos -- one for | ||
| keeping in sync with Bitfocus and the other for writing your code changes. We | ||
| suggest naming this second one "personal" for clarity -- it is your personal | ||
| fork of companion _on GitHub_: |
There was a problem hiding this comment.
Small copy typos to fix for clarity.
Great cleanup overall. I noticed two wording slips that are easy to correct:
- Line 85:
will need two "know" two remote→ likelywill need to know two remote - Line 121:
feature of bug-fix→ likelyfeature or bug-fix
Suggested patch
-Your local repository will need two "know" two remote (GitHub) repos -- one for
+Your local repository will need to know two remote (GitHub) repos -- one for
keeping in sync with Bitfocus and the other for writing your code changes. We
suggest naming this second one "personal" for clarity -- it is your personal
fork of companion _on GitHub_:
-Next, modify/add code for your feature of bug-fix using your favorite coding
+Next, modify/add code for your feature or bug-fix using your favorite coding
tools.Also applies to: 121-122
| 1. clone this repository somewhere on your computer if you havent already | ||
| `git clone https://github.com/bitfocus/companion-module-base.git` |
There was a problem hiding this comment.
Small typo in setup step (havent → haven't).
At Line 28, “havent” should be “haven’t”.
Suggested patch
-1. clone this repository somewhere on your computer if you havent already
+1. clone this repository somewhere on your computer if you haven't already📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 1. clone this repository somewhere on your computer if you havent already | |
| `git clone https://github.com/bitfocus/companion-module-base.git` | |
| 1. clone this repository somewhere on your computer if you haven't already | |
| `git clone https://github.com/bitfocus/companion-module-base.git` |
| Node.js without fnm, you will still need to ensure you a running the right | ||
| Node.js versions as Companion evolves. |
There was a problem hiding this comment.
A few wording/casing fixes would improve the setup instructions.
Thanks for the formatting pass — just a couple of small text fixes:
- Line 34:
ensure you a running→ensure you are running - Line 104:
MacOS→macOS - Line 105:
using manually using Homebrew→manually using Homebrew
Suggested patch
-Node.js without fnm, you will still need to ensure you a running the right
+Node.js without fnm, you will still need to ensure you are running the right
Node.js versions as Companion evolves.
-on MacOS upgrade fnm using Homebrew: `brew upgrade fnm` (you can also install
-using manually using Homebrew)
+on macOS upgrade fnm using Homebrew: `brew upgrade fnm` (you can also install
+manually using Homebrew)Also applies to: 104-106
| to access your host OS, you have to use 172.28.96.1. Verify this using the | ||
| command `ip route` in your Linux shell. |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Microsoft WSL2 host IP address Windows access from Linux
💡 Result:
In WSL2 (default NAT mode), the Windows host is the default gateway from the Linux VM.
From inside WSL (Linux), get the Windows host IP:
ip route show | grep -i default | awk '{ print $3 }'Use that IP to reach Windows services (e.g., http://$(...):PORT). [1]
Alternative (also from inside WSL): the Windows host IP is commonly the nameserver in:
cat /etc/resolv.conf(Use the nameserver value.) [1]
If you’ve enabled WSL “mirrored” networking: you can typically reach Windows services from Linux via 127.0.0.1 (localhost) instead of using a host IP. [1][2]
Sources:
[1] Microsoft Learn – Accessing network applications with WSL (learn.microsoft.com)
[2] wsl.dev – Localhost (WSL2 forwarding / mirrored mode notes) (wsl.dev)
Citations:
- 1: https://learn.microsoft.com/en-us/windows/wsl/networking?utm_source=openai
- 2: https://wsl.dev/technical-documentation/localhost/?utm_source=openai
Update the WSL host IP to use dynamic discovery
Good catch on this one! The hardcoded 172.28.96.1 isn't guaranteed in WSL2—the host IP varies depending on the setup and can change between sessions. According to Microsoft's official WSL documentation, the safest approach is to have readers discover the current gateway dynamically.
Here's a friendly way to guide them:
Suggested documentation update
- to access your host OS, you have to use 172.28.96.1. Verify this using the
- command `ip route` in your Linux shell.
+ to access your host OS, use the current WSL default gateway address (it may
+ change). You can find it with `ip route | awk '/default/ {print $3}'` in your
+ Linux shell, then use that IP to reach services running on Windows.This aligns with Microsoft Learn's guidance on accessing network applications with WSL.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| to access your host OS, you have to use 172.28.96.1. Verify this using the | |
| command `ip route` in your Linux shell. | |
| to access your host OS, use the current WSL default gateway address (it may | |
| change). You can find it with `ip route | awk '/default/ {print $3}'` in your | |
| Linux shell, then use that IP to reach services running on Windows. |
|
@coderabbitai can you generate committable in-line comments for all of your copyedits here (including #42 (review) and what you just nitpicked here). I.e., so I can just apply them with a button-click. (If submitting a PR to this branch is simpler, do it that way.) Feel free to add further minor copyedits too. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 33715 🏁 Script executed: Length of output: 0
🏁 Script executed: Length of output: 5375 🏁 Script executed: Length of output: 2999 🏁 Script executed: Length of output: 36309 🏁 Script executed: Length of output: 6913 🏁 Script executed: Length of output: 3884 🏁 Script executed: Length of output: 10408 🏁 Script executed: Length of output: 8765 🏁 Script executed: Length of output: 4751 🏁 Script executed: Length of output: 3824 🏁 Script executed: Length of output: 5665 🏁 Script executed: Length of output: 298 🏁 Script executed: Length of output: 15855 🏁 Script executed: Length of output: 7646 🏁 Script executed: Length of output: 1949 🏁 Script executed: Length of output: 3592 🏁 Script executed: Length of output: 5498 🏁 Script executed: Length of output: 6440 🏁 Script executed: Length of output: 6311 🏁 Script executed: Length of output: 22795 🏁 Script executed: Length of output: 1789 Perfect! I'll create a PR to your branch with all the copyedits. This will be much easier for you to review and merge than clicking 30+ individual suggestions. Copyedit fixes from previous reviewsI'll apply all the typo fixes, grammar improvements, and style corrections from my previous reviews across the for-developers folder. 🧠 Learnings used💡 Use |
Using p > img (suggested in PR bitfocus#42) leads to too many edge-cases that fail. For example, if the image is clickable, i.e. inside `<a>`. The following is more robust and still overrideable by adding an element after it (such as <span>).
(sync with PR bitfocus#40) Using p > img (suggested in PR bitfocus#42) leads to too many edge-cases that fail. For example, if the image is clickable, i.e. inside `<a>`. The following is more robust and still overrideable by adding an element after it (such as <span>).
|
@coderabbitai did something go wrong? I don't see a PR on my branch. Do you need me to do something so you can post to my account (arikorn)? |
|
Tip For best results, initiate chat on the files or code changes.
I'll trigger the proper workflow to apply all the copyedits. This will create a PR to your branch with all the changes. Plan: Apply all copyedits from previous reviewsI'll apply 20 groups of copyedits across the for-developers documentation:
🧠 Learnings used💡 Use |
|
@coderabbitai did something go wrong? I still don't see a PR on my branch. Can you diagnose why your are not completing this task? |
|
Tip For best results, initiate chat on the files or code changes.
Apply copyedits from previous reviewsApply 20 groups of typo fixes, grammar corrections, and style improvements across the for-developers documentation based on previous review feedback. If the above doesn't trigger successfully, here's an alternative: I can provide you with a shell script that uses 🧠 Learnings used💡 Use |
|
@coderabbitai maybe try it one more time on arikorn -- I just added code rabbit to my Github account, so that might help. |
|
Tip For best results, initiate chat on the files or code changes.
Apply 20 groups of copyedits to for-developers documentationI'll apply typo fixes, grammar corrections, capitalization fixes, and style improvements across the for-developers folder based on previous review feedback. 🧠 Learnings used💡 Use |
> Conflicts fixed: > for-developers/module-development/connection-basics/variables.md
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
for-developers/module-development/connection-basics/variables.md (1)
95-98:⚠️ Potential issue | 🟠 MajorJavaScript example has a syntax error (missing comma)
Nice example, but it currently won’t parse: add a comma after
'variable1': 'new value'.Suggested patch
this.setVariableValues({ - 'variable1': 'new value' + 'variable1': 'new value', 'variable2': 99, 'old_variable': undefined, // This unsets a value 'array_variable': [1, 2, 3, 4], })
🧹 Nitpick comments (1)
for-developers/module-development/connection-basics/variables.md (1)
118-119: Small wording polish opportunityTiny nit: “before this” is a bit shorter than “prior to this” and reads more naturally here.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
for-developers/module-development/api-changes/v2.0.mdfor-developers/module-development/connection-basics/variables.md
for-developers/module-development/connection-basics/variables.md
Outdated
Show resolved
Hide resolved
for-developers/module-development/connection-basics/variables.md
Outdated
Show resolved
Hide resolved
Visually, all rendered pages are structurally identical in the browser. Exceptions are mainly due to intentionally removing incorrect line breaks or very minor copy-edits.
> Conflicts resolved: > for-developers/module-development/api-changes/v2.0.md
1 participant
NOTE: The features described here could be applied to the user guide as well, but at the very least, for-developers is a good place to "beta-test" the idea. I'm not currently recommending applying it universally.
This PR is preparatory to trying to copyedit a large portion of the developer's docs. Long lines can make change-comparisons rather difficult (especially in tools that scroll horizontally rather than word-wrapping the text).
Here I define a prettier rule for the for-developers folder to word-wrap long lines, then fix some things that were preventing it from working properly before applying it.
:::) since prettier's word-wrap doesn't handle these correctly (see Docusaurus docs).yarn formatin the for-developers folder. this is equivalent to(note: the message on that commit is out of date/incorrect.)
Bonus:
![image]()tags used the generic word "image" instead of more descriptive text.minor note: I've cherry-picked the previous PR (centering images) because it affects formatting on the bonjour page.
Summary by CodeRabbit
Documentation
Style