Skip to content

docs: update outdated contribution guidelines #887

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 3 commits into from
Oct 19, 2024
Merged

docs: update outdated contribution guidelines #887

merged 3 commits into from
Oct 19, 2024

Conversation

cheton
Copy link
Collaborator

@cheton cheton commented Oct 19, 2024

No description provided.

Copy link

codesandbox bot commented Oct 19, 2024

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@cheton cheton linked an issue Oct 19, 2024 that may be closed by this pull request
13 tasks
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Outdated Node.js Version
The PR updates the development environment setup but doesn't specify a minimum Node.js version. This could lead to compatibility issues for contributors.

Incomplete Build Instructions
The updated build instructions for desktop apps don't include steps for Linux (ia32) and Windows (ia32) architectures, which were present in the original version.

Copy link
Contributor

codiumai-pr-agent-free bot commented Oct 19, 2024

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Score
Enhancement
Specify the minimum required Node.js version for clarity

Consider adding a note about the minimum required Node.js version for the project,
as it's not explicitly mentioned in the updated instructions.

CONTRIBUTING.md [112]

-Make sure you have the latest Node.js version installed, and run `yarn dev` to start a local development server for development and testing. Every code changes will trigger webpack Hot Module Replacement (HMR) which will be really useful while developing in React.
+Make sure you have Node.js version X.X or later installed. Run `yarn dev` to start a local development server for development and testing. Every code change will trigger webpack Hot Module Replacement (HMR), which is really useful while developing in React.
  • Apply this suggestion
Suggestion importance[1-10]: 7

Why:

7
Add brief explanations for each build command to improve clarity

Consider adding a brief explanation of what each build command does for different
platforms to provide more context to contributors.

CONTRIBUTING.md [142-164]

 #### macOS (x64)
 ```bash
+# Build for macOS x64 architecture
 $ yarn build && yarn build:macos-x64
 $ ls -al output/

macOS (arm64)

+# Build for macOS ARM64 architecture
$ yarn build && yarn build:macos-arm64
$ ls -al output/

Windows (x64)

+# Build for Windows x64 architecture
$ yarn build && yarn build:windows-x64
$ ls -al output/

Linux (x64)

+# Build for Linux x64 architecture
$ yarn build && yarn build:linux-x64
$ ls -al output/

- [ ] **Apply this suggestion** <!-- /improve --apply_suggestion=1 -->

<details><summary>Suggestion importance[1-10]: 7</summary>

Why: 

</details></details></td><td align=center>7

</td></tr></tr></tbody></table>

>💡 Need additional feedback ? start a [PR chat](https://chromewebstore.google.com/detail/ephlnjeghhogofkifjloamocljapahnl)

@cheton cheton merged commit 09a8b0d into master Oct 19, 2024
4 checks passed
@cheton cheton deleted the cncjs-860 branch October 19, 2024 11:45
cheton added a commit that referenced this pull request Oct 20, 2024
* docs: update outdated contribution guidelines

* chore: update webpack.config.development.js

* docs: update CONTRIBUTING.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Outdated instructions on cncjs github installation
1 participant