Skip to content

Comments

docs: Added dash to all samples#1051

Merged
orhun merged 9 commits intoorhun:mainfrom
okydk:patch-1
Feb 24, 2025
Merged

docs: Added dash to all samples#1051
orhun merged 9 commits intoorhun:mainfrom
okydk:patch-1

Conversation

@okydk
Copy link
Contributor

@okydk okydk commented Feb 20, 2025

Description

Docs were inconsistent and gave errors when copy and pasting.
Looked like a polyfill for git, which I guess isn't the intention.

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (no code change)
  • Refactor (refactoring production code)
  • Other

@okydk okydk requested a review from orhun as a code owner February 20, 2025 11:34
@welcome
Copy link

welcome bot commented Feb 20, 2025

Thanks for opening this pull request! Please check out our contributing guidelines! ⛰️

@codecov-commenter
Copy link

codecov-commenter commented Feb 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 41.94%. Comparing base (5f3a3d0) to head (fb840b8).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1051   +/-   ##
=======================================
  Coverage   41.94%   41.94%           
=======================================
  Files          21       21           
  Lines        1786     1786           
=======================================
  Hits          749      749           
  Misses       1037     1037           
Flag Coverage Δ
unit-tests 41.94% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@orhun
Copy link
Owner

orhun commented Feb 20, 2025

Hello, good catch!

Docs were inconsistent and gave errors when copy and pasting.

What kind of errors did you get? Normally using either of git-cliff or git cliff would work, as long as you have git installed in your system (which I assume so).

I agree that there might be inconsistencies in the docs and we should fix them. It's a matter of which style we want to go for I guess :)

@okydk
Copy link
Contributor Author

okydk commented Feb 21, 2025

Arh ok, I've mostly run in through npx git-cliff directly, where it certainly doesn't work without the dash. :)

@orhun
Copy link
Owner

orhun commented Feb 21, 2025

Ah, certainly. Both forms of git-cliff and git cliff would work on the CLI but npx should call git-cliff explicitly. Maybe we can add a note to https://git-cliff.org/docs/installation/npm instead of updating the other parts of the documentation?

@okydk
Copy link
Contributor Author

okydk commented Feb 21, 2025

Sure, it's your decision at the end of the day. :)

Just wanted to help out and make it more consistant, as many of the examples that uses both syntaxes looks like typos.
And since git-cliff is the only one that works cross all environments I would vouch for that one.

@orhun
Copy link
Owner

orhun commented Feb 21, 2025

I personally like the git cliff usage, since it's more aligned with the rest of the git commands. (e.g. git status, git log, etc.)

Instead of updating usages, I'd prefer we add a note about:

  1. git-cliff is the correct invocation, however calling git cliff is also possible when git is installed.
  2. When using npx, git-cliff should be used.

Would you be down to update this PR based on this? 😊

@okydk
Copy link
Contributor Author

okydk commented Feb 21, 2025

Cool, I respect that. Yes, will update the PR. Great tool btw! 🙌

@orhun
Copy link
Owner

orhun commented Feb 21, 2025

Great, thank you <3 Ping me whenever this is ready for a review :)

@okydk
Copy link
Contributor Author

okydk commented Feb 24, 2025

I've reverted to minor changes. 😊 Should be ready now.

Copy link
Owner

@orhun orhun left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@orhun orhun merged commit cd26bb2 into orhun:main Feb 24, 2025
68 of 69 checks passed
@welcome
Copy link

welcome bot commented Feb 24, 2025

Congrats on merging your first pull request! ⛰️

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