From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id B51012B9E; Mon, 4 Mar 2019 17:59:27 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5E7DC307D874; Mon, 4 Mar 2019 16:59:26 +0000 (UTC) Received: from localhost.localdomain (unknown [10.18.25.8]) by smtp.corp.redhat.com (Postfix) with ESMTP id 86B0C5D6B3; Mon, 4 Mar 2019 16:59:25 +0000 (UTC) To: Thomas Monjalon Cc: dev@dpdk.org, stable@dpdk.org, Bruce Richardson , Van Haaren Harry , ramirose@gmail.com References: <20190214193547.30783-1-msantana@redhat.com> <7663396.UzxFzaq1Qb@xps> <5bfdfd39-8d04-dc88-ccc1-16ba9377f9ec@redhat.com> <2081114.IUId4rgEkx@xps> From: Michael Santana Francisco Organization: Red Hat Message-ID: <03b7bb23-3627-e12a-1a8a-1b6f0e557514@redhat.com> Date: Mon, 4 Mar 2019 11:59:25 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: <2081114.IUId4rgEkx@xps> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.48]); Mon, 04 Mar 2019 16:59:26 +0000 (UTC) Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH v3 1/2] Enable codespell by default. Can be disabled from config file. X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list Reply-To: msantana@redhat.com List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 04 Mar 2019 16:59:28 -0000 On 3/1/19 4:08 PM, Thomas Monjalon wrote: > 01/03/2019 21:24, Michael Santana Francisco: >> On 3/1/19 12:51 PM, Thomas Monjalon wrote: >>> 01/03/2019 18:43, Thomas Monjalon: >>>> 01/03/2019 18:08, Michael Santana: >>>>> +# Enable codespell by default. This can be overwritten from a config file. >>>>> +# You can also enable codespell by setting DPDK_CHECKPATCH_CODESPELL to a valid path >>>>> +# to a dictionary.txt file if your dictionary.txt is not in the default location. >>> This line length won't pass chekpatch ;) >>> >>>> Better to avoid "you" form in such comment. >>>> >>>>> +DPDK_CHECKPATCH_CODESPELL=enable >>> It will override the value if passed with an environment variable. >>> You should do the same as for DPDK_CHECKPATCH_LINE_LENGTH. >> If I understood you correctly, you want to be able to set these >> parameters via environment variables (and take precedence over any >> variables set in any config file) > No :) > The scenario is to have no config file and use environment variables only. > >> The problem is right now that any environment variable is overwritten by >> the variable being set in one of the config files >> The only way I can think of doing this would be by saving the DPDK >> variables (passed via environment) to a file or ironically temporary >> variables (which themselves can also be overwritten, so that doesn't >> really solve the problem) before being overwritten, and then restoring >> said variables after the call to source. >> This would add extra clutter in checkpatches, but it can be avoided by >> doing it in load-devel-config instead. >> >> So the bottom line is, environment variables take overall precedence, >> then config files, and then default >> >> Does this sound sane enough? >> If anyone knows a better way to do this please share. > Look how DPDK_CHECKPATCH_LINE_LENGTH is handled. > The default value is used if DPDK_CHECKPATCH_LINE_LENGTH is not set, > neither by environment nor config file. > > I think you can just do this after loading config file: > > DPDK_CHECKPATCH_CODESPELL=${DPDK_CHECKPATCH_CODESPELL:-enable} Oh! So it's much simpler than I thought. Yeah. I will remove what I have now and put in your line to go along with the length= line. Will also change the comment as I didn't remove the 'you' form in the last commit > > or check for empty value in the test: > > [ -z "$DPDK_CHECKPATCH_CODESPELL" -o "$DPDK_CHECKPATCH_CODESPELL" = enable ] > > >> I am including in DPDK_CHECKPATCH_PATH, because might as well at this >> point. > Empty DPDK_CHECKPATCH_PATH is already handled. > >>>>> # Load config options: >>>>> # - DPDK_CHECKPATCH_PATH >>>>> # - DPDK_CHECKPATCH_LINE_LENGTH >>>>> +# - DPDK_CHECKPATCH_CODESPELL >>>>> . $(dirname $(readlink -e $0))/load-devel-config >>>>> >>>>> VALIDATE_NEW_API=$(dirname $(readlink -e $0))/check-symbol-change.sh >>>>> @@ -13,6 +18,12 @@ length=${DPDK_CHECKPATCH_LINE_LENGTH:-80} >>>>> >>>>> # override default Linux options >>>>> options="--no-tree" >>>>> +if [ "$DPDK_CHECKPATCH_CODESPELL" == "enable" ]; then >>>> This is a bashism. >>>> Standard sh uses a simple = >>>> >>>> No need for a v4, I can fix it. >>> Because of the required change for the env var case, >>> please do a v4. > >