-
Notifications
You must be signed in to change notification settings - Fork 59
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
YETUS-1145. add rstcheck #267
base: main
Are you sure you want to change the base?
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.
Should the i18n change come via a separate JIRA? Seems like it touches more code than was expected from the rstcheck feature.
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.
ant, cmake, gradel, maven, make all follow the --foo-cmd
pattern.
|
||
for i in "$@"; do | ||
case ${i} in | ||
--rstcheck=*) |
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.
This should be --rstcheck-cmd
, so as to be consistent with our other plugins that permit specifying the executable path?
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.
precommit is wildly inconsistent here. Some use --foo-cmd=
and some use --foo=
. There should really be a follow-on JIRA to make these consistent.
I wanted to cover rstcheck support for i18n while I was still thinking about it. Now that the i18n PR is merged, I've rebased on top of the current main in order to pull it in. |
No description provided.