ALT Linux Team development discussions
 help / color / mirror / Atom feed
From: "Vladimir D. Seleznev" <vseleznv@altlinux.org>
To: ALT Linux Team development discussions <devel@lists.altlinux.org>
Subject: Re: [devel] [PATCH v4 1/2] Add diff exclude option for gear-rules
Date: Mon, 7 Dec 2020 20:57:07 +0300
Message-ID: <20201207175707.GC1074869@portlab> (raw)
In-Reply-To: <20201207165809.GA13984@altlinux.org>

On Mon, Dec 07, 2020 at 07:58:09PM +0300, Dmitry V. Levin wrote:
> On Fri, Nov 27, 2020 at 04:44:30PM +0300, Vladimir D. Seleznev wrote:
> > The option specifies a glob pattern that define files that should be
> 
> pattern that defineS
> 
> > excluded from diff generation.
> > 
> > Signed-off-by: Vladimir D. Seleznev <vseleznv@altlinux.org>
> > ---
> >  gear            | 17 +++++++++++++----
> >  gear-rules.5.in | 19 +++++++++++++++++++
> >  gear-store-tags |  4 ++++
> >  3 files changed, 36 insertions(+), 4 deletions(-)
> > 
> > diff --git a/gear b/gear
> > index 6282c92..7bdddc0 100755
> > --- a/gear
> > +++ b/gear
> > @@ -337,7 +337,7 @@ make_archive()
> >  	write_git_archive "$format" "$optional" "$cmd" "$tar_tree" "$dir_name" "$tar_name" "$tar_base" "$tar_suffix"
> >  }
> >  
> > -get_diff_name()
> > +get_diff_opts()
> >  {
> >  	local old_d="$1" && shift
> >  	local new_d="$1" && shift
> 
> OK
> 
> > @@ -376,6 +376,13 @@ get_diff_name()
> >  				;;
> >  			name=*) diff_name="${opt#name=}"
> >  				;;
> > +			exclude=*)
> > +				[ -z "$(printf %s "${opt#exclude=}" |tr -d '[:alnum:]_.?*-/![]')" ] ||
> > +					rules_error "Invalid diff exclude pattern specified: ${opt#exclude=}"
> 
> Why '[:alnum:]_.?*-/![]' ?
> 
> > +				[ -n "${opt#exclude=}" ] ||
> > +					rules_error "Invalid diff exclude pattern specified: empty pattern"
> > +				diff_exclude="$diff_exclude :^${opt#exclude=}"
> > +				;;
> 
> You're doing this ${opt#exclude=} thing 4 times,
> I suggest saving the result to a variable and using it afterwards.

https://lists.altlinux.org/pipermail/devel/2020-November/212766.html

On Fri, Nov 27, 2020 at 02:28:08PM +0100, Alexey Gladkov wrote:
> Здесь и в gear-store-tags не требуется заводить новую переменную только
> для того чтобы переложить $opt без префикса в $diff_exclude.
> 
> С другими опциями это оправдано так как значения этих опций используются
> в функции ниже.
> 
> Предлагаю написать просто: opt="${opt#exclude=}"

-- 
   WBR,
   Vladimir D. Seleznev


      parent reply	other threads:[~2020-12-07 17:57 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-27 13:44 Vladimir D. Seleznev
2020-11-27 13:44 ` [devel] [PATCH v4 2/2] tests: add test for diff exclude option Vladimir D. Seleznev
2020-12-07 14:30 ` [devel] [PATCH v4 1/2] Add diff exclude option for gear-rules Vladimir D. Seleznev
2020-12-07 16:58 ` Dmitry V. Levin
2020-12-07 17:07   ` Alexey Gladkov
2020-12-07 17:19     ` Dmitry V. Levin
2020-12-07 17:29       ` Alexey Gladkov
2020-12-07 17:57   ` Vladimir D. Seleznev [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=20201207175707.GC1074869@portlab \
    --to=vseleznv@altlinux.org \
    --cc=devel@lists.altlinux.org \
    /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

ALT Linux Team development discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://lore.altlinux.org/devel/0 devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 devel devel/ http://lore.altlinux.org/devel \
		devel@altlinux.org devel@altlinux.ru devel@lists.altlinux.org devel@lists.altlinux.ru devel@linux.iplabs.ru mandrake-russian@linuxteam.iplabs.ru sisyphus@linuxteam.iplabs.ru
	public-inbox-index devel

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://lore.altlinux.org/org.altlinux.lists.devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git