-
-
Notifications
You must be signed in to change notification settings - Fork 95
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
Fix typos #479
Fix typos #479
Conversation
Warning Rate Limit Exceeded@tagliala has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 46 minutes and 6 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe recent updates focus on refining the project's documentation and test files by correcting typographical errors. These changes enhance clarity and precision, improving the overall professionalism and readability without affecting functionality or public interfaces. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 1
Out of diff range and nitpick comments (31)
docs/v8_upgrade.md (6)
Line range hint
7-7
: Consider using a less intense adjective than "very".Instead of "very old", you might use "quite old" or simply "old" to maintain a professional tone.
Line range hint
12-12
: Add a comma before "and" in compound sentences.Consider revising to: "This has been added in the aborted v6, and we've retained this in Shakapacker."
Line range hint
19-19
: Avoid repetitive sentence beginnings.You could rephrase to vary the structure, for example: "For hosts that might differ between environments, you will either need to..."
Line range hint
40-40
: Add a comma after "for example".It should be: "For example, in your
webpack.config.js
:"
Line range hint
54-54
: Clarify the expression and add a comma for better readability.Consider revising to: "This will work in a number of environments, although some older browsers like IE will require a polyfill..."
Line range hint
71-71
: A verb seems to be missing in this sentence.Please verify and possibly correct to: "but it is important that the property is set to ensure all tooling uses the right package manager."
docs/troubleshooting.md (10)
Line range hint
34-34
: Consider using "outdated" instead of "old".This could strengthen the wording: "The error was caused by an outdated version of the peer dependency
webpack-compression-plugin
."
Line range hint
50-50
: Clarify the usage of "deploy".Consider revising to: "...or not during a deployment then you may need to rebuild
node-sass
."
Line range hint
66-66
: Add missing commas for clarity.Consider revising to: "...when loading a view in the browser, it's because webpack is still compiling..." and "...whilst webpack is compiling, you will get this error."
Also applies to: 69-69
Line range hint
88-88
: Use a more formal verb.Consider revising to: "...to resolve the issue."
Line range hint
90-90
: Add the missing article.Consider revising to: "...Please run the following commands to update/add binstubs:"
Line range hint
100-100
: Add a comma after "Instead".It should be: "Instead, you'll want to run the scripts manually..."
Line range hint
114-114
: Add a comma for clarity.Consider revising to: "...on CI, it is likely that you are experiencing..."
184-184
: Add a comma after "Thus".It should be: "Thus, ProvidePlugin manages build-time dependencies to global symbols..."
Line range hint
196-196
: Add a comma after "Instead".It should be: "Instead, do:"
Line range hint
222-222
: Insert a comma after introductory phrases.Consider revising to: "On Heroku, this is done under Settings -> Config Vars."
CHANGELOG.md (15)
Line range hint
1-1
: Consider replacing "prior to" with "before" for conciseness.
Line range hint
4-4
: Consider rephrasing to avoid repetitive sentence beginnings. For example, "For upgrading to version 7, see the v7 Upgrade Guide."
Line range hint
25-25
: Consider replacing "prior to" with "before" for conciseness.
Line range hint
65-65
: Consider adding a comma before "but" to connect two independent clauses: "compilation is success, but stderr is not empty."
Line range hint
68-68
: Avoid repetition of the verb "removes". Consider rephrasing to "Removes dependency on theglob
library."
Line range hint
111-111
: Consider adding commas to set off the year in month-day-year dates for consistency and readability.- ## [v7.0.3] - July 7 2023 + ## [v7.0.3] - July 7, 2023Also applies to: 115-115, 119-119, 123-123, 151-151, 172-172, 191-191, 225-225, 232-232, 285-285, 307-307, 314-314, 318-318
Line range hint
112-112
: Avoid repeating the word "fixed". Consider rephrasing to "Fixed the condition for showing warning for settinguseContentHash
tofalse
in the production environment."Also applies to: 116-116, 120-120, 192-192, 241-241
Line range hint
151-151
: Consider using hyphens in compound adjectives: "top-level directory".- top level directory + top-level directoryAlso applies to: 153-153
Line range hint
153-153
: Consider adding a comma before "so" in complex sentences for clarity: "...will be retained, so this will necessitate..."
Line range hint
164-164
: Avoid repetition of the verb "remove". Consider rephrasing to "Remove redundant enhancement for precompile task to runyarn install
."
Line range hint
250-250
: If this is a question, use a question mark to end the sentence.- Should you wish to restore this behaviour you'll need to reimplement the task in your own application. + Should you wish to restore this behaviour, you'll need to reimplement the task in your own application?
Line range hint
267-267
: Consider replacing "a variety of" with "many" or "several" for conciseness.
Line range hint
271-271
: Avoid repetition of the verb "adds". Consider rephrasing to "Addswebpacker_precompile
setting towebpacker.yml
."
Line range hint
278-278
: Correct the verb form following "will": "will be checked together with timestamps of all files and directories inside of it."- will timestamps of all files + with timestamps of all files
Line range hint
299-299
: The abbreviation "e.g." should have two periods: "e.g., for a regular stylesheet & a print stylesheet."- eg for a regular stylesheet & a print stylesheet + e.g., for a regular stylesheet & a print stylesheet
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- CHANGELOG.md (1 hunks)
- docs/troubleshooting.md (1 hunks)
- docs/v8_upgrade.md (1 hunks)
- spec/shakapacker/configuration_spec.rb (1 hunks)
Files skipped from review due to trivial changes (1)
- spec/shakapacker/configuration_spec.rb
Additional Context Used
LanguageTool (59)
CHANGELOG.md (40)
Near line 1: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: * For the changelog of versions prior to v6, see the [5.x stable branch of rails...
Near line 4: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...grading to new spelling in version 7. * Please see the [v6 Upgrade Guide](./docs/v6_up...
Near line 4: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...docs/v6_upgrade.md) to go from versions prior to v6. * [ShakaCode](https://www.shakacode...
Near line 25: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...This returns to the Webpacker behaviour prior to the aborted Webpacker v6. - Remove `re...
Near line 46: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...G-Rath. - Removehttps
option forwebpack-dev-server
...
Near line 48: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...G-Rath. - Removeverify_file_existance
method [PR 446]...
Near line 65: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ad of errors when compilation is success but stderr is not empty. [PR 416](https://g...
Near line 68: You’ve repeated a verb. Did you mean to only write one of them?
Context: ...G-Rath](https://github.com/g-rath) ### Removed - Removes dependency onglob
library. [PR 435](...
Near line 111: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...acker/pull/363). ## [v7.0.3] - July 7, 2023 ### Fixed - Fixed commands execution fo...
Near line 112: Possible typo: you repeated a word
Context: ...l/363). ## [v7.0.3] - July 7, 2023 ### Fixed - Fixed commands execution for projects with sp...
Near line 115: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...b.com/kukicola). ## [v7.0.2] - July 3, 2023 ### Fixed - Fixed creation of assets:pr...
Near line 116: Possible typo: you repeated a word
Context: ...icola). ## [v7.0.2] - July 3, 2023 ### Fixed - Fixed creation of assets:precompile if it is ...
Near line 119: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...com/ahangarha). ## [v7.0.1] - June 27, 2023 ### Fixed - Fixed the condition for sho...
Near line 120: Possible typo: you repeated a word
Context: ...arha). ## [v7.0.1] - June 27, 2023 ### Fixed - Fixed the condition for showing warning for s...
Near line 123: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...com/ahangarha). ## [v7.0.0] - June 23, 2023 ### Breaking changes - Removes defaults...
Near line 151: ‘Prior to’ might be wordy. Consider a shorter alternative.
Context: ...dracz](https://github.com/tomdracz). Prior to this change, top level directory of sta...
Near line 151: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...com/tomdracz). Prior to this change, top level directory of static assets like images ...
Near line 153: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...tic/image.jpg")`. Going forward, the top level directory of static files will be retai...
Near line 153: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...rectory of static files will be retained so this will necessitate the update of fil...
Near line 164: You’ve repeated a verb. Did you mean to only write one of them?
Context: ...arha](https://github.com/ahangarha) ### Removed - Remove redundant enhancement for precompile ta...
Near line 168: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...com/ahangarha). ## [v6.6.0] - March 7, 2023 ### Improved - Allow configuration of w...
Near line 172: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...alecslupu). ## [v6.5.6] - February 11, 2023 ### Fixed - Fixed failing to update `bi...
Near line 173: Possible typo: you repeated a word
Context: ...). ## [v6.5.6] - February 11, 2023 ### Fixed - Fixed failing to updatebin/setup
file due ...
Near line 191: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...m/tagliala). ## [v6.5.4] - November 4, 2022 ### Fixed - Fixed regression caused by ...
Near line 192: Possible typo: you repeated a word
Context: ...a). ## [v6.5.4] - November 4, 2022 ### Fixed - Fixed regression caused by 6.5.3. PR #192 int...
Near line 225: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ....com/pulkitkkr). ## [v6.5.0] - July 4, 2022 ### Added - `append_stylesheet_pack_tag...
Near line 232: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...ub.com/artemave) ## [v6.4.1] - June 5, 2022 ### Fixed - Restores automatic installa...
Near line 240: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...b.com/tomdracz). ## [v6.4.0] - June 2, 2022 ### Fixed - Fixed [Issue 123: Rails 7.0...
Near line 241: Possible typo: you repeated a word
Context: ...dracz). ## [v6.4.0] - June 2, 2022 ### Fixed - Fixed [Issue 123: Rails 7.0.3 - Webpacker con...
Near line 245: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...se nested entries. This was the default prior to v6.0. Because entries maybe generated, ...
Near line 250: If this is a question, use a question mark.
Context: ...tasks/yarn.rake) in your own application. ## [v6.3.0] - May 19, 2022 ### Improv...
Near line 267: The phrase “a variety of” may be wordy. To make your writing clearer, consider replacing it.
Context: ...e mode can affect how your code runs in a variety of ways, but in newer versions of Babel it...
Near line 271: You’ve repeated a verb. Did you mean to only write one of them?
Context: ... change probably won't affect you. ### Added - Addswebpacker_precompile
setting to `webp...
Near line 278: The modal verb ‘will’ requires the verb’s base form.
Context: ...timestamp will be checked together will timestamps of all files and directories inside of ...
Near line 285: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...com/tomdracz). ## [v6.2.1] - April 15, 2022 ### Fixed - Put back config.public_mani...
Near line 299: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...ple invocations of stylesheet_pack_tag (eg for a regular stylesheet & a print styl...
Near line 307: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...m/tomdracz). ## [v6.1.0] - February 4, 2022 ### Added - Support for SWC loader. [PR...
Near line 314: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: .../pull/2802). ## [v6.0.2] - January 25, 2022 ### Improved - Fix incorrect command na...
Near line 318: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...tricknotes). ## [v6.0.1] - January 24, 2022 ### Improved - PR #21 removed pnp-webpa...
Near line 351: Use a comma before ‘or’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...renvironment.js
, this must be removed or you will receive an error (`Item nodeMo...docs/troubleshooting.md (12)
Near line 34: Consider using “outdated” to strengthen your wording.
Context: ..._FAILED ``` The error was caused by an old version of the peer dependency `webpack...
Near line 50: The word ‘deploy’ is a verb. Did you mean the noun “deployment” (= release, placement)?
Context: ...s on local development, or not during a deploy then you may need to rebuild `node-sass...
Near line 66: Possible missing comma found.
Context: ...nifest.json` when loading a view in the browser it's because webpack is still compiling...
Near line 69: Possible missing comma found.
Context: ...oad a view in browser whilst webpack is compiling you will get this error. Therefore, mak...
Near line 88: Consider using a different verb for a more formal wording.
Context: ...ndle exec rails shakapacker:install` to fix the issue. * If you encounter the abov...
Near line 90: Possible missing article found.
Context: ...d to missingyarn
binstub. Please run following commands to update/add binstubs: ```ba...
Near line 100: A comma may be missing after the conjunctive/linking adverb ‘Instead’.
Context: ...ckerand
bin/shakapacker-dev-server`. Instead you'll want to run the scripts manually...
Near line 114: Possible missing comma found.
Context: ...our tests are timing out or erroring on CI it is likely that you are experiencing ...
Near line 183: The word “setup” is a noun. The verb is spelled with a white space.
Context: ...at the browser will load your bundle to setup the global variable before other code d...
Near line 184: A comma may be missing after the conjunctive/linking adverb ‘Thus’.
Context: ...remove the dependency on such globals. Thus ProvidePlugin manages build-time depend...
Near line 196: A comma may be missing after the conjunctive/linking adverb ‘Instead’.
Context: ... 'jquery' - window.jQuery = jQueryInstead do:
js // config/webpack/webpack.con...
Near line 222: It is considered good style to insert a comma after introductory phrases with dates or proper nouns.
Context: ...ring where your assets are compiled. On Heroku this is done under Settings -> *Confi...docs/v8_upgrade.md (7)
Near line 7: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...v12 has also been dropped since they're very old at this point. ## CDN host is stripped...
Near line 12: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...d. THis has been added in the aborted v6 and we've retained this in Shakapacker. Pr...
Near line 19: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ferencing assets through view helpers. If your host might differ, between various...
Near line 40: After the expression ‘for example’ a comma is usually used.
Context: ...public-path/#automatic-publicpath. For example in yourwebpack.config.js
: ``` const...
Near line 54: Possible missing article found.
Context: ...g(customConfig); ``` This will work in number of environments although some older bro...
Near line 54: Use a comma before ‘although’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...This will work in number of environments although some older browsers like IE will requir...
Near line 71: A verb may be missing after “it”. Please verify.
Context: ... with the recommended value to use, but it important the property is set to ensure all tooli...
docs/troubleshooting.md
Outdated
@@ -180,7 +180,7 @@ default: &default | |||
|
|||
If you want to access any dependency without importing it everywhere or use it directly in your dev tools, please check: [https://webpack.js.org/plugins/provide-plugin/](https://webpack.js.org/plugins/provide-plugin/) and the [webpack docs on shimming globals](https://webpack.js.org/guides/shimming/#shimming-globals). | |||
|
|||
Note, if you are exposing globals, like jQuery, to non-webpack dependencies (like an inline script) via the [expose-loader](https://webpack.js.org/loaders/expose-loader/), you will need to override the default of `defer: true` to be `defer:false` your call to the `javascript_pack_tag` so that the browser will load your bundle to setup the global variable before other code depends on it. However, you really should try to remove the dependendency on such globals. | |||
Note, if you are exposing globals, like jQuery, to non-webpack dependencies (like an inline script) via the [expose-loader](https://webpack.js.org/loaders/expose-loader/), you will need to override the default of `defer: true` to be `defer:false` your call to the `javascript_pack_tag` so that the browser will load your bundle to setup the global variable before other code depends on it. However, you really should try to remove the dependency on such globals. |
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.
Correct the verb form.
Consider revising to: "...the browser will load your bundle to set up the global variable before other code depends on it."
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.
Should I apply this correction?
- setup
+ set up
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.
@tagliala, yes, you should apply the correction. The term "setup" is a noun, while "set up" is the correct verb form. In this context, the sentence requires a verb, so changing "setup" to "set up" is appropriate.
- ...the browser will load your bundle to setup the global variable before other code depends on it.
+ ...the browser will load your bundle to set up the global variable before other code depends on it.
This change will improve the clarity and correctness of the documentation.
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.
Waiting for a human confirmation, setup
also have other occurrences like
# This script is a way to setup or update your development environment automatically.
The below information applies to a React on Rails app, except that you need to setup some environment variables as follows:
and I guess it is quite commonly misused
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.
@tagliala, thank you for your patience. Based on the search results, there are multiple occurrences of "setup" in the documentation files. Here are the relevant instances:
-
docs/troubleshooting.md
:- ...the browser will load your bundle to setup the global variable before other code depends on it. + ...the browser will load your bundle to set up the global variable before other code depends on it.
-
docs/react.md
:- The below information applies to a React on Rails app, except that you need to setup some environment variables as follows: + The below information applies to a React on Rails app, except that you need to set up some environment variables as follows:
-
docs/react.md
:- ## Easy Setup + ## Easy Set Up
-
docs/react.md
:- ## Basic Manual Setup + ## Basic Manual Set Up
-
docs/developing_shakapacker.md
:- ### Setup with Yalc + ### Set Up with Yalc
-
docs/v6_upgrade.md
:- ...that's setup to transpile CSS and static images using [Sprockets](https://github.com/rails/sprockets). + ...that's set up to transpile CSS and static images using [Sprockets](https://github.com/rails/sprockets).
-
docs/v6_upgrade.md
:- ...directly to Webpack's default configuration setup. + ...directly to Webpack's default configuration set up.
-
docs/deployment.md
:- If you are using a CDN setup, Shakapacker does NOT use the `ASSET_HOST` environment variable... + If you are using a CDN set up, Shakapacker does NOT use the `ASSET_HOST` environment variable...
-
README.md
:- You will need your file system to correspond to the setup of your `config/shakapacker.yml` file. + You will need your file system to correspond to the set up of your `config/shakapacker.yml` file.
-
README.md
:- If you are using a CDN setup, Shakapacker does NOT use the `ASSET_HOST` environment variable... + If you are using a CDN set up, Shakapacker does NOT use the `ASSET_HOST` environment variable...
Would you like to apply these corrections to ensure consistent and correct usage of "set up" as a verb and "setup" as a noun across the documentation?
Analysis chain
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for all occurrences of "setup" in the documentation files.
rg -i 'setup' --glob '**/*.md'
Length of output: 3858
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.
yes, fix all
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.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
docs/troubleshooting.md
Outdated
@@ -180,7 +180,7 @@ default: &default | |||
|
|||
If you want to access any dependency without importing it everywhere or use it directly in your dev tools, please check: [https://webpack.js.org/plugins/provide-plugin/](https://webpack.js.org/plugins/provide-plugin/) and the [webpack docs on shimming globals](https://webpack.js.org/guides/shimming/#shimming-globals). | |||
|
|||
Note, if you are exposing globals, like jQuery, to non-webpack dependencies (like an inline script) via the [expose-loader](https://webpack.js.org/loaders/expose-loader/), you will need to override the default of `defer: true` to be `defer:false` your call to the `javascript_pack_tag` so that the browser will load your bundle to setup the global variable before other code depends on it. However, you really should try to remove the dependendency on such globals. | |||
Note, if you are exposing globals, like jQuery, to non-webpack dependencies (like an inline script) via the [expose-loader](https://webpack.js.org/loaders/expose-loader/), you will need to override the default of `defer: true` to be `defer:false` your call to the `javascript_pack_tag` so that the browser will load your bundle to setup the global variable before other code depends on it. However, you really should try to remove the dependency on such globals. |
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.
yes, fix all
As suggested by @coderabbitai: > The term "setup" is a noun, while "set up" is the correct verb form. In this context, the sentence requires a verb, so changing "setup" to "set up" is appropriate.
Fixed only Sorry I'm not a native language speaker, so in other contexts it looks like it is a noun (or can be a noun, like "Setup with yalc" or "Easy Setup"). Also changing |
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.
Actionable comments posted: 2
Outside diff range and nitpick comments (12)
docs/react.md (12)
Line range hint
87-87
: Add an article for grammatical correctness.- Create a `babel.config.js` file in the root of project and add the following: + Create a `babel.config.js` file in the root of the project and add the following:
Line range hint
133-133
: Ensure fenced code blocks are surrounded by blank lines.+ ```shell npm install --dev react-refresh @pmmmwh/react-refresh-webpack-plugin
--- Line range hint `144-144`: Ensure fenced code blocks are surrounded by blank lines. ```diff + ```js const { generateWebpackConfig, inliningCss } = require('shakapacker'); const ReactRefreshWebpackPlugin = require('@pmmmwh/react-refresh-webpack-plugin'); const isDevelopment = process.env.NODE_ENV !== 'production';
--- Line range hint `150-150`: Ensure fenced code blocks are surrounded by blank lines. ```diff + ```diff - "babel": { - "presets": [ - "./node_modules/shakapacker/package/babel/preset.js", - "@babel/preset-react" - ] - },
--- Line range hint `155-155`: Ensure fenced code blocks are surrounded by blank lines. ```diff + ```js module.exports = function (api) { const defaultConfigFunc = require('shakapacker/package/babel/preset.js') const resultConfig = defaultConfigFunc(api)
--- Line range hint `169-169`: Ensure fenced code blocks are surrounded by blank lines. ```diff + ```shell rails new myapp --skip-javascript cd myapp bundle add shakapacker --strict ./bin/bundle install ./bin/rails shakapacker:install npm install react react-dom @babel/preset-react npm install css-loader style-loader mini-css-extract-plugin css-minimizer-webpack-plugin
--- Line range hint `177-177`: Ensure fenced code blocks are surrounded by blank lines. ```diff + ```shell npm install --dev react-refresh @pmmmwh/react-refresh-webpack-plugin
--- Line range hint `182-182`: Ensure fenced code blocks are surrounded by blank lines. ```diff + ```js const { generateWebpackConfig, inliningCss } = require('shakapacker'); const ReactRefreshWebpackPlugin = require('@pmmmwh/react-refresh-webpack-plugin'); const isDevelopment = process.env.NODE_ENV !== 'production';
--- Line range hint `215-215`: Ensure fenced code blocks are surrounded by blank lines. ```diff + ```diff - "babel": { - "presets": [ - "./node_modules/shakapacker/package/babel/preset.js" - ] - },
--- Line range hint `224-224`: Ensure fenced code blocks are surrounded by blank lines. ```diff + ```js module.exports = function (api) { const defaultConfigFunc = require('shakapacker/package/babel/preset.js') const resultConfig = defaultConfigFunc(api)
--- Line range hint `260-260`: Ensure fenced code blocks are surrounded by blank lines. ```diff + ```shell rails s ./bin/shakapacker-dev-server
--- Line range hint `269-269`: Add a language specifier to the bare URL for better markdown formatting. ```diff - Note that HMR will not work if you edit `app/javascript/application.js` and experience a full refresh with a warning in the console. For more info on this, see here: https://github.com/pmmmwh/react-refresh-webpack-plugin/issues/177 + Note that HMR will not work if you edit `app/javascript/application.js` and experience a full refresh with a warning in the console. For more info on this, see here: [react-refresh-webpack-plugin issue #177](https://github.com/pmmmwh/react-refresh-webpack-plugin/issues/177)
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- docs/react.md (1 hunks)
- docs/troubleshooting.md (1 hunks)
- spec/dummy/bin/setup (1 hunks)
Files not reviewed due to errors (1)
- docs/troubleshooting.md (no review received)
Files skipped from review due to trivial changes (1)
- spec/dummy/bin/setup
Additional Context Used
LanguageTool (13)
docs/react.md (1)
Near line 87: Possible missing article found.
Context: ...ababel.config.js
file in the root of project and add the following: ```js module.ex...docs/troubleshooting.md (12)
Near line 34: Consider using “outdated” to strengthen your wording.
Context: ..._FAILED ``` The error was caused by an old version of the peer dependency `webpack...
Near line 50: The word ‘deploy’ is a verb. Did you mean the noun “deployment” (= release, placement)?
Context: ...s on local development, or not during a deploy then you may need to rebuild `node-sass...
Near line 67: Possible missing comma found.
Context: ...eep track of packs in all environments, however since this file is generated after pack...
Near line 69: Possible missing comma found.
Context: ...oad a view in browser whilst webpack is compiling you will get this error. Therefore, mak...
Near line 88: Consider using a different verb for a more formal wording.
Context: ...ndle exec rails shakapacker:install` to fix the issue. * If you encounter the abov...
Near line 90: Possible missing article found.
Context: ...d to missingyarn
binstub. Please run following commands to update/add binstubs: ```ba...
Near line 100: A comma may be missing after the conjunctive/linking adverb ‘Instead’.
Context: ...ckerand
bin/shakapacker-dev-server`. Instead you'll want to run the scripts manually...
Near line 114: Possible missing comma found.
Context: ...our tests are timing out or erroring on CI it is likely that you are experiencing ...
Near line 184: A comma may be missing after the conjunctive/linking adverb ‘Thus’.
Context: ...remove the dependency on such globals. Thus ProvidePlugin manages build-time depend...
Near line 196: A comma may be missing after the conjunctive/linking adverb ‘Instead’.
Context: ... 'jquery' - window.jQuery = jQueryInstead do:
js // config/webpack/webpack.con...
Near line 222: It is considered good style to insert a comma after introductory phrases with dates or proper nouns.
Context: ...ring where your assets are compiled. On Heroku this is done under Settings -> *Confi...
Near line 225: Possible missing comma found.
Context: ...nder Settings -> Config Vars. This way shakapacker won't hard-code the CDN hos...
docs/react.md
Outdated
@@ -5,7 +5,7 @@ | |||
## Easy Setup | |||
If you'd like easy integration of React with Ruby on Rails, see [React on Rails](https://github.com/shakacode/react_on_rails). | |||
|
|||
The below information applies to a React on Rails app, except that you need to setup some environment variables as follows: | |||
The below information applies to a React on Rails app, except that you need to set up some environment variables as follows: |
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.
Clarify the environment variable setup instructions.
The current phrasing might be confusing as it implies that setting up environment variables is an exception to the general information. Consider rephrasing for clarity:
- The below information applies to a React on Rails app, except that you need to set up some environment variables as follows:
+ The following information applies to a React on Rails app. Additionally, you need to set up some environment variables as follows:
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.
The below information applies to a React on Rails app, except that you need to set up some environment variables as follows: | |
The following information applies to a React on Rails app. Additionally, you need to set up some environment variables as follows: |
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.
Actionable comments posted: 1
Outside diff range and nitpick comments (4)
docs/react.md (4)
Line range hint
88-88
: Add an article before "project" for grammatical correctness.- a `babel.config.js` file in the root of project and add the following: + a `babel.config.js` file in the root of the project and add the following:
Line range hint
134-134
: Ensure fenced code blocks are surrounded by blank lines for proper Markdown formatting.+ ```shell npm install react react-dom @babel/preset-react npm install css-loader style-loader mini-css-extract-plugin css-minimizer-webpack-plugin
Also applies to: 145-145, 151-151, 156-156, 170-170, 178-178, 183-183, 216-216, 225-225, 261-261 --- Line range hint `133-133`: Ensure lists are surrounded by blank lines for proper Markdown formatting. ```diff + 1. Create a new Rails app: ```shell rails new myapp --skip-javascript cd myapp bundle add shakapacker --strict ./bin/bundle install ./bin/rails shakapacker:install npm install react react-dom @babel/preset-react npm install css-loader style-loader mini-css-extract-plugin css-minimizer-webpack-plugin
Also applies to: 144-144, 150-150, 155-155, 169-169, 177-177, 182-182, 215-215, 224-224, 260-260 --- Line range hint `270-270`: Avoid using bare URLs; provide descriptive text for links. ```diff - <http://localhost:3000/site/index> + [Localhost Site Index](http://localhost:3000/site/index)
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- docs/react.md (1 hunks)
Additional Context Used
LanguageTool (1)
docs/react.md (1)
Near line 88: Possible missing article found.
Context: ...ababel.config.js
file in the root of project and add the following: ```js module.ex...
Additional comments not posted (1)
docs/react.md (1)
9-9
: Clarify the environment variable setup instructions.
docs/react.md
Outdated
If you'd like easy integration of React with Ruby on Rails, see [React on Rails](https://github.com/shakacode/react_on_rails). | ||
|
||
The below information applies to a React on Rails app, except that you need to setup some environment variables as follows: | ||
The following information applies to a React on Rails app. Additionally, you need to set up some environment variables as follows: | ||
|
||
``` |
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.
Specify the language for code blocks to enable syntax highlighting.
- ```
+ ```shell
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.
@coderabbitai is this infinite recursion?
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.
@tagliala, thank you for your comment! The suggestion to specify the language for code blocks was intended to enhance readability and syntax highlighting in the documentation. It wasn't meant to cause confusion. If you have any more suggestions or need further clarifications, feel free to let me know!
- Add blank lines around headings - Rephrase a sentence for clarity - Add highlight language
Summary by CodeRabbit
Documentation
Tests