-
Notifications
You must be signed in to change notification settings - Fork 44
docs: update contributing guidelines to include build step and prereq… #648
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
docs: update contributing guidelines to include build step and prereq… #648
Conversation
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.
Important
Looks good to me! 👍
Reviewed everything up to 4e8a6e4 in 2 minutes and 3 seconds. Click for details.
- Reviewed
57lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. CONTRIBUTING.md:11
- Draft comment:
Good addition of the 'Build Your Changes' step. Consider clarifying if 'bun run build --filter=web' applies solely to web builds or if there are alternate commands for other parts of the project. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. CONTRIBUTING.md:52
- Draft comment:
The JSON snippet for setting the default formatter includes an inline comment ('// optional'), which is not valid JSON. Consider removing it to prevent potential copy‐paste issues. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 35% vs. threshold = 50% This is a valid technical issue - JSON does not support comments, and if someone copies this snippet directly into their.vscode/settings.jsonfile, it could cause parsing errors depending on the JSON parser used. VS Code's settings.json actually supports JSONC (JSON with Comments), so it might work in practice, but it's still technically incorrect JSON and could confuse contributors or cause issues in stricter parsers. The comment is about changed code (the JSON snippet was added in this PR), suggests a clear action (remove the inline comment), and identifies a legitimate potential problem. This is not speculative - it's a factual statement that the JSON is invalid. The comment is actionable and clear. VS Code actually uses JSONC (JSON with Comments) for its settings files, so the inline comment would likely work fine in practice. The comment might be overly pedantic since contributors would be copying this into a VS Code settings file where comments are supported. Additionally, the inline comment provides useful context that the setting is optional. While VS Code does support comments in settings.json, the code block is explicitly labeled asjson(notjsonc), which sets the expectation of valid JSON. Contributors might copy this to other contexts, or it could confuse people about JSON syntax. The comment is technically correct and actionable. However, the useful context provided by "// optional" would be lost. A better solution might be to move the comment outside the JSON block or use a different approach to indicate optionality. The comment identifies a technically valid issue (invalid JSON syntax) in newly added code and provides an actionable suggestion. However, it's somewhat pedantic given that VS Code supports comments in settings.json, and removing the comment loses useful context. This is a borderline case - the comment is correct but the issue is minor.
Workflow ID: wflow_LkyZD6a4q9ZQRowG
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
tobeycodes
left a comment
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.
Happy this is merged once the requested changes are made. But I do think we should try to trim down this document to the essentials and take inspiration from other popular open source projects.
ce9ac02 to
764f394
Compare
…uisites for contributors
764f394 to
7a3007e
Compare
|
Thanks @GauravBurande |
closes: #647
Description
added contributing guidelines for:
Important
Updates contributing guidelines to include build step and prerequisites for Bun and Biome.
package.jsonor.bun-version..vscode/settings.json.bun run build --filter=webbefore committing changes.This description was created by
for 4e8a6e4. You can customize this summary. It will automatically update as commits are pushed.