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.
prev parent 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