[
https://issues.apache.org/jira/browse/YETUS-191?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15033160#comment-15033160
]
Allen Wittenauer edited comment on YETUS-191 at 12/1/15 5:57 AM:
-----------------------------------------------------------------
I think the general idea of a verify function to be a good one! But it raises
some interesting questions that I need to think and play around with it a bit
more though. e.g., is test-patch.sh the correct place? Maybe it should be in
core? would smart-apply-patch benefit? etc
While I think a bit more about that, a few relatively minor nits (and some bike
shedding ;) ):
* the @return value is missing and @replaceable is listed twice
* If cmd_path is empty, I'd really like to know that (specifically) cmd_name's
executable wasn't specified. Otherwise I'd be "errr, yes, it was. I told you
to do this plug-in on the command line!"
{code}
+ if [[ -z ${cmd_path} ]]; then
+ yetus_error "${cmd_name} is not specified."
{code}
* This really makes me think the parameter order is awkward and it could very
well be because perlcritic's error message was awkward to start with. hah.
{code}
+ if ! verify_command "${PERLCRITIC}" "Perl::Critic" perlcritic; then
{code}
* if we're going to add this function, then findbugs and docker (and probably
others) should also be using it. I think findbugs, in particular, will clarify
what this interface should look like since it's a much more complicated missing
exe output.
was (Author: aw):
I think the general idea of a verify command to be a good one! But it raises
some interesting questions that I need to think and play around with it a bit
more though. e.g., is test-patch.sh the correct place? Maybe it should be in
core? would smart-apply-patch benefit? etc
While I think a bit more about that, a few relatively minor nits (and some bike
shedding ;) ):
* the @return value is missing and @replaceable is listed twice
* If cmd_path is empty, I'd really like to know that (specifically) cmd_name's
executable wasn't specified. Otherwise I'd be "errr, yes, it was. I told you
to do this plug-in on the command line!"
{code}
+ if [[ -z ${cmd_path} ]]; then
+ yetus_error "${cmd_name} is not specified."
{code}
* This really makes me think the parameter order is awkward and it could very
well be because perlcritic's error message was awkward to start with. hah.
{code}
+ if ! verify_command "${PERLCRITIC}" "Perl::Critic" perlcritic; then
{code}
* if we're going to add this function, then findbugs and docker (and probably
others) should also be using it. I think findbugs, in particular, will clarify
what this interface should look like since it's a much more complicated missing
exe output.
> ruby-lint and rubocop have broken output on error
> -------------------------------------------------
>
> Key: YETUS-191
> URL: https://issues.apache.org/jira/browse/YETUS-191
> Project: Yetus
> Issue Type: Bug
> Components: Test Patch
> Reporter: Allen Wittenauer
> Assignee: Marco Zühlke
> Priority: Trivial
> Attachments: YETUS-191.00.patch, YETUS-191.01.patch
>
>
> Output on console is missing exe names if they aren't installed.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
