Skip to content

docs(cli): add dev docs and update#1046

Open
lwasser wants to merge 1 commit into
all-contributors:mainfrom
lwasser:docs-cli2
Open

docs(cli): add dev docs and update#1046
lwasser wants to merge 1 commit into
all-contributors:mainfrom
lwasser:docs-cli2

Conversation

@lwasser

@lwasser lwasser commented Mar 8, 2026

Copy link
Copy Markdown
Member

This is a reado of #1032 which i'll close

Checklist:

  • Documentation
  • Ready to be merged
  • Added myself to contributors table.
    Bot Usage

@netlify

netlify Bot commented Mar 8, 2026

Copy link
Copy Markdown

Deploy Preview for all-contributors ready!

Name Link
🔨 Latest commit f6b8a34
🔍 Latest deploy log https://app.netlify.com/projects/all-contributors/deploys/69ace4283bca2f0008821242
😎 Deploy Preview https://deploy-preview-1046.staging.allcontributors.org
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@lwasser
lwasser requested a review from JimMadge March 8, 2026 02:51
@lwasser

lwasser commented Mar 8, 2026

Copy link
Copy Markdown
Member Author

ok @JimMadge, I'd suggest for my PR"s in the future, don't hit the update branch button because I'll rebase locally. Once a merge commit is in the mix, things get very messy and difficult to clean up. I prefer the rebase as it's cleaner. But for other contributors, we can update the branch right before we merge, and it should be ok. Maybe we can read about this more when you squash-merge, and there's a merge commit instead of a rebase. I don't know how that works.

@lwasser

lwasser commented Mar 8, 2026

Copy link
Copy Markdown
Member Author

When I run the linter locally, the README file is failing. But I didn't touch the README file, so I'd rather that get fixed outside of this pr which is just focused on CLI docs. We could turn this into an issue if needed.

@JimMadge

JimMadge commented Mar 9, 2026

Copy link
Copy Markdown
Member

ok @JimMadge, I'd suggest for my PR"s in the future, don't hit the update branch button because I'll rebase locally. Once a merge commit is in the mix, things get very messy and difficult to clean up. I prefer the rebase as it's cleaner. But for other contributors, we can update the branch right before we merge, and it should be ok. Maybe we can read about this more when you squash-merge, and there's a merge commit instead of a rebase. I don't know how that works.

Oh, sorry for giving you trouble. I didn't use the update branch button. It wasn't available because there were conflicts with the target branch. I did a merge manually (mainly because I prefer merges and don't like having to fix conflicts multiple times in a rebase 😆).

Is there a problem with rebasing a branch after a merge commit? Maybe git has trouble knowing which parent to use? Maybe you can squash the branch then rebase? In any case, sorry, I didn't foresee that.

Does this branch include the changes from the comments in the PR? were there any left unresolved?

@JimMadge

JimMadge commented Mar 9, 2026

Copy link
Copy Markdown
Member

Regarding semantic line breaks, we don't say anything about expecting or enforcing that. But the existing documentation is in that style, so I would say it is better for consistency.

I won't block this PR because of that though, so I suppose that is a soft ask from me 😆.

@JimMadge JimMadge left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming this is as the last PR with the suggestions, good 👍.

There are some linting problems with the new cli development file. Once those are resolved I think we can merge. The other questions aren't important enough to block anything.

@JimMadge

JimMadge commented Mar 9, 2026

Copy link
Copy Markdown
Member

When I run the linter locally, the README file is failing. But I didn't touch the README file, so I'd rather that get fixed outside of this pr which is just focused on CLI docs. We could turn this into an issue if needed.

I can't reproduce this locally. What was the command you ran and package versions?

@trueberryless trueberryless left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like the changes in here. Awesome work!

I think that the content from the new development.md file could also fit in a new CONTRIBUTING.md file, which is the standard in FOSS to get new developers to know the codebase and tools we use.

I created another PR which also cleans up and refactors the documentation a lot, we might wanna merge that first to get a clean state and then apply your PR on that, wdyt?

| `npm run test-coverage` | Run tests with coverage and generate reports |
| `npm run commit` | Commit changes using a commitizen-compatible interface |
| `npm run knip` | Find unused files, dependencies, and exports |
| `npm run add-contributor` | Add a contributor |

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess to satisfy the linter, you would need to align the column end:

Suggested change
| `npm run add-contributor` | Add a contributor |
| `npm run add-contributor` | Add a contributor |

To run the test suite use:

```bash
npm test

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this command intended for this repository? Because I can't find any Vitest setup here. This might need an update...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants