Skip to content
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

Document when one would want to use --staged #244

Merged
merged 2 commits into from
Nov 9, 2022

Conversation

scop
Copy link
Contributor

@scop scop commented Nov 11, 2021

I was wondering why I'd need to set user.name and user.email when running gitlint with pre-commit in CI, and realized it's because of --staged in the hook's default config. It wasn't immediately clear to me why I'd want --staged (turns out I don't), so I thought I'd document it.

Might be nice to also document that with --staged one needs those git configs set, but don't know if that's too much detail on this level.

scop added a commit to scop/pytekukko that referenced this pull request Nov 11, 2021
We don't need it as we're not checking anything besides the commit
message, and using it requires user.name and user.email to be set in
config, which is a minor annoyance in CI.

Refs jorisroovers/gitlint#244
scop added a commit to scop/pytekukko that referenced this pull request Nov 11, 2021
We don't need it as we're not checking anything besides the commit
message, and using it requires user.name and user.email to be set in
config, which is a minor annoyance in CI.

Refs jorisroovers/gitlint#244
@jorisroovers
Copy link
Owner

Thanks for the suggestion. I can see that --staged maybe isn't the most prescriptive CLI flag.

I do think the sentence "Useful if checking other properties of a commit besides just the message." doesn't necessarily tell much more than the existing "Read staged commit meta-info from the local repository". I guess the word "meta-info" is where you got caught up?

Maybe we should change it to:

"Read staged commit meta-info (username, email, changed files, etc) from the local repository".

That would also keep the --help output concise. We can then still add more text to the docs to explain it better.

@scop
Copy link
Contributor Author

scop commented Nov 13, 2021

To me the interesting thing is why would I want to have staged commit meta-info read from the repo. In that sense, "if checking other properties of a commit besides just the message" is to the point and helpful to me.

On the other hand, the "(username, email, changed files, etc)" addition does add some detail, but leaves the question kind of open -- without having a look under the hood, I wouldn't know if gitlint needed that info for something not directly related to which properties of a commit I want to lint, or if I can leave it out.

But yeah, my suggestion isn't as concise as I'd like it to be, and it feels kind of silly to repeat it everywhere. The reason I added it to both --help and the config docs is that I could not decide which would be the place I'd look first for more info on the option. If I'd look at --help, it clearly would need to be there, and if I'd look at the web docs, I'd probably notice the same thing in the main page and I'm not sure if I'd search for more info elsewhere.

scop added a commit to scop/pylttoaine that referenced this pull request Dec 3, 2021
We don't need it as we're not checking anything besides the commit
message, and using it requires user.name and user.email to be set in
config, which is a minor annoyance in CI.

Refs jorisroovers/gitlint#244
@jorisroovers jorisroovers added the docs Documentation related items label Oct 27, 2022
@jorisroovers jorisroovers self-assigned this Oct 27, 2022
@jorisroovers jorisroovers added this to the 0.18.0 milestone Oct 27, 2022
scop and others added 2 commits November 9, 2022 08:29
@jorisroovers jorisroovers merged commit 64fe9a2 into jorisroovers:main Nov 9, 2022
@jorisroovers
Copy link
Owner

Added a more lengthy explanation in the docs and updated the --help output to something I believe is more descriptive. Thanks for bringing this up :)

@scop scop deleted the staged-docs branch November 12, 2022 07:10
jorisroovers added a commit that referenced this pull request Nov 16, 2022
- Python 3.11 support
- Last release to support Python 3.6
- Behavior Change: In a future release, gitlint will be switching to use
  `re.search` instead of `re.match` semantics for all rules. (#254)
- gitlint no longer uses the `sh` library by default in an attempt to reduce
  external dependencies.
- `--commits` now also accepts a comma-separated list of commit hashes,
  making it possible to lint a list of non-contiguous commits without invoking
  gitlint multiple times (#283)
- Improved handling of branches that have no commits (#188)
- Support for `GITLINT_CONFIG` env variable (#189)
- Added a new `gitlint-ci` pre-commit hook, making it easier to run gitlint
  through pre-commit in CI (#191)
- Contrib Rules:
  - New `contrib-disallow-cleanup-commits` rule (#312)
  - New `contrib-allowed-authors` rule (#358)
- User Defined rules:
  - Gitlint now recognizes `fixup=amend` commits, available as
    `commit.is_fixup_amend_commit=True`
  - Gitlint now parses diff **stat** information, available in
    `commit.changed_files_stats` (#314)
- Bugfixes:
  - Use correct encoding when using `--msg-filename` parameter (#310)
  - Various documentation fixes (#244) (#263) (#266) (#294) (#295) (#347) (#364)
- Under-the-hood:
  - Dependencies updated
  - Moved to blacked for formatting
  - Fixed nasty CI issue (#298)
  - Unit tests fix (#256)
  - Vagrant box removed in favor of github dev containers (#348)
  - Removed a few lingering references to the `master` branch in favor of `main`
  - Moved roadmap and project planning to github projects

Full Release details in CHANGELOG.md.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation related items
Projects
Status: 0.18.0
Development

Successfully merging this pull request may close these issues.

None yet

2 participants