[PATCH] abrt-cli status: fix the output

[Denys Vlasenko]

Before:
ABRT has detected '2' problem(s). (For more info run: $ abrt-cli list --since 1372083129 )
After:
ABRT has detected 2 problem(s). For more info run: abrt-cli list --since 1372083129

Signed-off-by: Denys Vlasenko 
---
 src/cli/status.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/cli/status.c b/src/cli/status.c
index 08045b9..17a1865 100644
--- a/src/cli/status.c
+++ b/src/cli/status.c
@@ -102,7 +102,7 @@ int cmd_status(int argc, const char **argv)
         else
         {
             char *list_arg = opt_since ? xasprintf(" --since %d", opt_since) : xstrdup("");
-            printf(_("ABRT has detected '%u' problem(s). (For more info run: $ abrt-cli list%s )\n"), problem_count, list_arg);
+            printf(_("ABRT has detected %u problem(s). For more info run: abrt-cli list%s\n"), problem_count, list_arg);
             free(list_arg);
         }
     }

[Jakub Filak]

Is this patch related to some ticket? Or why do you think that the patch
fixes the output?

Regards
Jakub

On Mon, 2013-06-24 at 16:24 +0200, Denys Vlasenko wrote:
> Before:
> ABRT has detected '2' problem(s). (For more info run: $ abrt-cli list --since 1372083129 )
> After:
> ABRT has detected 2 problem(s). For more info run: abrt-cli list --since 1372083129
> Signed-off-by: Denys Vlasenko
>
> src/cli/status.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/cli/status.c b/src/cli/status.c
> index 08045b9..17a1865 100644
> --- a/src/cli/status.c
> +++ b/src/cli/status.c
> @@ -102,7 +102,7 @@ int cmd_status(int argc, const char **argv)
>          else
>          {
>              char *list_arg = opt_since ? xasprintf(" --since %d", opt_since) : xstrdup("");
>        printf(_("ABRT has detected '%u' problem(s). (For more info run: $ abrt-cli list%s )\n"), problem_count, list_arg);
>        printf(_("ABRT has detected %u problem(s). For more info run: abrt-cli list%s\n"), problem_count, list_arg);
>        free(list_arg);
>    }
> }

[Denys Vlasenko]

On 06/24/2013 04:50 PM, Jakub Filak wrote:
> Is this patch related to some ticket?

No, it's a result of me looking at the output and thinking
"for me as a user, this message looks wrong".
> Or why do you think that the patch
> fixes the output?

Because:
- numbers are not enclosed in quotes in English language
- assuming that user has '$' as shell prompt is unwarranted

[Richard Marko]

On 06/24/2013 04:56 PM, Denys Vlasenko wrote:
> On 06/24/2013 04:50 PM, Jakub Filak wrote:
> > Is this patch related to some ticket?
>
> No, it's a result of me looking at the output and thinking
> "for me as a user, this message looks wrong".
> > Or why do you think that the patch
> > fixes the output?
>
> Because:
> numbers are not enclosed in quotes in English language
> assuming that user has '$' as shell prompt is unwarranted

+1

This patch makes it better.

[Jakub Filak]

On Mon, 2013-06-24 at 17:24 +0200, Richard Marko wrote:
> On 06/24/2013 04:56 PM, Denys Vlasenko wrote:
> > On 06/24/2013 04:50 PM, Jakub Filak wrote:
> > > Is this patch related to some ticket?
> >
> > No, it's a result of me looking at the output and thinking
> > "for me as a user, this message looks wrong".
> > > Or why do you think that the patch
> > > fixes the output?
> >
> > Because:
> > numbers are not enclosed in quotes in English language
> > assuming that user has '$' as shell prompt is unwarranted
>
> +1
>
> This patch makes it better.

Sure, it looks much better.

I was just unhappy with the commit message because I don't know when we
have to reference a ticket in the commit message. I would say that if a
patch needs review then the patch also needs an reference to some
ticket. We should update the developer guide. Or am I completely wrong?

[Jiri Moskovcak]

On 06/26/2013 09:07 AM, Jakub Filak wrote:
> On Mon, 2013-06-24 at 17:24 +0200, Richard Marko wrote:
> > On 06/24/2013 04:56 PM, Denys Vlasenko wrote:
> > > On 06/24/2013 04:50 PM, Jakub Filak wrote:
> > > > Is this patch related to some ticket?
> > >
> > > No, it's a result of me looking at the output and thinking
> > > "for me as a user, this message looks wrong".
> > > > Or why do you think that the patch
> > > > fixes the output?
> > >
> > > Because:
> > > numbers are not enclosed in quotes in English language
> > > assuming that user has '$' as shell prompt is unwarranted
> >
> > +1
> >
> > This patch makes it better.
>
> Sure, it looks much better.
>
> I was just unhappy with the commit message because I don't know when we
> have to reference a ticket in the commit message. I would say that if a
> patch needs review then the patch also needs an reference to some
> ticket. We should update the developer guide. Or am I completely wrong?

Let's keep it simple:
- review: always
- ticket: if it's not trivial (in cases where it's not clear, it's up to 
reviewer to decide)

--Jirka

[Jakub Filak]

Pushed. Thanks!

On Mon, 2013-06-24 at 16:24 +0200, Denys Vlasenko wrote:
> Before:
> ABRT has detected '2' problem(s). (For more info run: $ abrt-cli list --since 1372083129 )
> After:
> ABRT has detected 2 problem(s). For more info run: abrt-cli list --since 1372083129
> Signed-off-by: Denys Vlasenko
>
> src/cli/status.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/cli/status.c b/src/cli/status.c
> index 08045b9..17a1865 100644
> --- a/src/cli/status.c
> +++ b/src/cli/status.c
> @@ -102,7 +102,7 @@ int cmd_status(int argc, const char **argv)
>          else
>          {
>              char *list_arg = opt_since ? xasprintf(" --since %d", opt_since) : xstrdup("");
>        printf(_("ABRT has detected '%u' problem(s). (For more info run: $ abrt-cli list%s )\n"), problem_count, list_arg);
>        printf(_("ABRT has detected %u problem(s). For more info run: abrt-cli list%s\n"), problem_count, list_arg);
>        free(list_arg);
>    }
> }