[jira] [Comment Edited] (YETUS-191) ruby-lint and rubocop have broken output on error

["Allen Wittenauer (JIRA)" <ji...@apache.org>]

    [ 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)