Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladimir Davydov <vdavydov.dev@gmail.com>
To: Roman Khabibov <roman.habibov@tarantool.org>
Cc: tarantool-patches@freelists.org
Subject: Re: [PATCH v3] fio: cleanup error messages
Date: Wed, 12 Dec 2018 16:58:06 +0300	[thread overview]
Message-ID: <20181212135806.vmiyuncwxrbeoxdj@esperanza> (raw)
In-Reply-To: <20181211224614.6609-1-roman.habibov@tarantool.org>

On Wed, Dec 12, 2018 at 01:46:14AM +0300, Roman Khabibov wrote:
> Otherwise it is hard to debug code, throwing
> exceptions. fio.pathjoin was just one example among
> many.
> 
> Closes #3580
> 
> Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-3580-err-msg-pathjoin
> Issue: https://github.com/tarantool/tarantool/issues/3580

Branch/Issue prefixes are not necessary - it's clear which is which
without them. Also, the links should go after the '---' below (this way
they are ignored by git-am). And you forgot a changelog - whenever you
resubmit a patch after making some changes you should write a brief
change log, e.g.

https://www.freelists.org/post/tarantool-patches/PATCH-v3-replication-fix-exit-with-ER-NO-SUCH-USER-during-bootstrap

> ---
>  src/lua/fio.lua       | 18 ++++++-------
>  test/app/fio.result   | 61 +++++++++++++++++++++++++++++++++++++++++++
>  test/app/fio.test.lua | 19 +++++++++++++-
>  3 files changed, 88 insertions(+), 10 deletions(-)
> 
> diff --git a/src/lua/fio.lua b/src/lua/fio.lua
> index 55faebdcb..f92267023 100644
> --- a/src/lua/fio.lua
> +++ b/src/lua/fio.lua
> @@ -126,7 +126,7 @@ fio_methods.seek = function(self, offset, whence)
>      end
>      if type(whence) == 'string' then
>          if fio.c.seek[whence] == nil then
> -            error(sprintf("Unknown whence: %s", whence))
> +            error(sprintf("Usage: fio.seek() unknown whence: %s", whence))

I meant that "Usage: ..." pattern should only be used for errors that
don't have any additional details. Sorry for misunderstanding. I amended
it by myself.

> +-- gh-3580: Check that error messages are descriptive enough.
> +fh1:seek(nil, 'a')
> +---
> +- error: 'builtin/fio.lua:129: Usage: fio.seek() unknown whence: a'
> +...

Forgot to tell you last time. These error messages would change if we
made any changes to fio.lua, which would be annoying. I filtered the
line numbers out (see test_run:cmd("push filter ...")).

>  fio.unlink(tmp1)
>  fio.unlink(tmp2)
> -fio.rmdir(tmpdir)
> +fio.rmdir(tmpdir)
> \ No newline at end of file

Newline was removed by your patch. Please fix your editor. I amended it
as well and pushed the patch to 2.1.

      reply	other threads:[~2018-12-12 13:58 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-11 22:46 Roman Khabibov
2018-12-12 13:58 ` Vladimir Davydov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181212135806.vmiyuncwxrbeoxdj@esperanza \
    --to=vdavydov.dev@gmail.com \
    --cc=roman.habibov@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [PATCH v3] fio: cleanup error messages' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox