-
Notifications
You must be signed in to change notification settings - Fork 101
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
Ignore stdin by default in CI #423
Comments
We can do this yes (not sure whether this is the right implementation, need to check). However, is the implication here that gitlint doesn't work out of the box in your CI environment without specifying |
That's what concerns me as I haven't needed to ignore stdin at all |
I haven't come across a case where I would have actually needed it, rather just decided to follow gitlint's documentation on it being useful in (some?) CI :) |
So this should really be "improve documentation to indicate that this might be useful in CI"? |
Would be good to be more specific than that. I'm not sure what audience would such a change serve, and there wouldn't be much of a difference to the current situation. Alternatively, the note about CI could be removed altogether, I suppose people who need it will either find the option or not, no matter if CI is mentioned. |
Sorry for the delay - and thanks for opening issues and your doc fix PRs @scop, please keep them coming. We might not always move forward with everything, but the discussion and suggestions make gitlint better overall - greatly appreciated! On topic: So if we don't know of any CI environment where detection fails, I think I'm now undecided on the usefulness of this. I agree with using sane defaults ("batteries included" ftw!) but you could also argue it's better to have a hard fail on TTY/STDIN detection (and people opening a bug) than it silently being bypassed because
Are you referring to the mention of CI here, in the CLI flag? Or do you mean elsewhere in the gitlint docs? gitlint/gitlint-core/gitlint/cli.py Line 270 in 1f8a16b
|
Both. There's a mention at least in docs/configuration.md (marked "sometimes useful" there). |
I'm not aware of a setup where it would be actually required. Refs jorisroovers/gitlint#423
I'm not aware of a setup where it would be actually required. Refs jorisroovers/gitlint#423
One of the nice things of nox are good defaults, no matter if it's run in CI or not.
I thought it'd make sense for gitlint, too. Following the nox implementation, something like this could do the trick:
Perhaps some additional patching for the test suite would be in order, as it's likely to run also in CI itself, but in principle. Doc updates obviously too.
The text was updated successfully, but these errors were encountered: