test suite reviews and discussions
 help / color / mirror / Atom feed
From: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
To: "ohilyard@iol.unh.edu" <ohilyard@iol.unh.edu>,
	"dts@dpdk.org" <dts@dpdk.org>
Cc: "lijuan.tu@intel.com" <lijuan.tu@intel.com>
Subject: Re: [dts] [PATCH v2] ci/initial: Added script to get the tests for a patchset
Date: Fri, 3 Sep 2021 09:42:44 +0000	[thread overview]
Message-ID: <35cfdcbff7f54d9d9e00d611d1490db8@pantheon.tech> (raw)
In-Reply-To: <20210812153647.74262-1-ohilyard@iol.unh.edu>



> -----Original Message-----
> From: dts <dts-bounces@dpdk.org> On Behalf Of ohilyard@iol.unh.edu
> Sent: Thursday, August 12, 2021 5:37 PM
> To: dts@dpdk.org
> Cc: lijuan.tu@intel.com; Owen Hilyard <ohilyard@iol.unh.edu>
> Subject: [dts] [PATCH v2] ci/initial: Added script to get the tests for a patchset

I'd strongly suggest using these guidelines (and also adding them to the DTS WG requirements) for writing commit messages: https://chris.beams.io/posts/git-commit/

I think DPDK also uses those (or very similar) guidelines. We'll be consistent and the just rules make sense.

In this commit message I think only the form in the subject should be changed to imperative.

> 
> From: Owen Hilyard <ohilyard@iol.unh.edu>
> 
> This script should be run after the patchset has been applied.
> It will check all files that have a diff to the git ref in DTS_MAIN_BRANCH_REF
> (currently origin/master).
> 
> It will also issue warnings to standard error if a "protected path" is changed. This
> is currently configured to only by the ci scripts folder, since under most
> circumstances a patch should not need to change anything in there. This warning
> will be in the format:
> "WARNING: {file_name} is protected"
> 
> The script will also issue a warning if a config file is changed. This warning is also
> sent to standard error and takes the form of:
> "WARNING: {file_name} is a config file and was changed"
> 
> The script will output a list of the test suites to run to standard out, with each
> entry having one line.
> 
> Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> ---
>  ci/Dockerfile                |  18 ++++
>  ci/README.txt                |  37 +++++++
>  ci/build_image.sh            |   0
>  ci/get_tests_for_patchset.py | 197 +++++++++++++++++++++++++++++++++++
>  ci/requirements.txt          |  32 ++++++
>  5 files changed, 284 insertions(+)
>  create mode 100644 ci/Dockerfile
>  create mode 100644 ci/README.txt
>  create mode 100644 ci/build_image.sh
>  create mode 100644 ci/get_tests_for_patchset.py  create mode 100644
> ci/requirements.txt
> 
> diff --git a/ci/Dockerfile b/ci/Dockerfile new file mode 100644 index
> 00000000..89645b36
> --- /dev/null
> +++ b/ci/Dockerfile
> @@ -0,0 +1,18 @@
> +# This container should be build in the ci directory, and then the #
> +DTS directory should be mounted as a volume at /dts/ FROM
> +python:3.9-slim-buster
> +
> +ENV DEBIAN_FRONTEND=noninteractive
> +
> +COPY requirements.txt .
> +COPY dts_requirements.txt dts_requirements.txt
> +
> +RUN apt-get update && apt-get install --no-install-recommends -y \
> +    # Add a C compiler for all of the c modules in DTS
> +    build-essential make gcc git libpcap-dev\
> +    python3-pip
> +
> +RUN pip3 install -r requirements.txt
> +RUN pip3 install -r dts_requirements.txt
> +# install formatter
> +RUN pip3
> +install black
> \ No newline at end of file

Are these missing newlines intentional?

> diff --git a/ci/README.txt b/ci/README.txt new file mode 100644 index
> 00000000..281329f7
> --- /dev/null
> +++ b/ci/README.txt
> @@ -0,0 +1,37 @@
> +# BSD LICENSE
> +#
> +# Copyright(c) 2021 University of New Hampshire Interoperability Laboratory.
> All rights reserved.
> +# All rights reserved.
> +#
> +# Redistribution and use in source and binary forms, with or without #
> +modification, are permitted provided that the following conditions #
> +are met:
> +#
> +#   * Redistributions of source code must retain the above copyright
> +#     notice, this list of conditions and the following disclaimer.
> +#   * Redistributions in binary form must reproduce the above copyright
> +#     notice, this list of conditions and the following disclaimer in
> +#     the documentation and/or other materials provided with the
> +#     distribution.
> +#   * Neither the name of Intel Corporation nor the names of its
> +#     contributors may be used to endorse or promote products derived
> +#     from this software without specific prior written permission.
> +#

Should Intel be still mentioned here?

In general, how should the licence look like? Was there a discussion about this?

<snip>

> +argparse==1.4.0
> \ No newline at end of file

The other newline and the reason for plural earlier. Pointing this out in case we'll change it.

> --
> 2.30.2
> 



  reply	other threads:[~2021-09-03  9:42 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-12 15:36 ohilyard
2021-09-03  9:42 ` Juraj Linkeš [this message]
2021-09-03 16:49   ` Owen Hilyard
2021-10-13 14:19     ` ohilyard
2022-04-06 13:01       ` Tu, Lijuan

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=35cfdcbff7f54d9d9e00d611d1490db8@pantheon.tech \
    --to=juraj.linkes@pantheon.tech \
    --cc=dts@dpdk.org \
    --cc=lijuan.tu@intel.com \
    --cc=ohilyard@iol.unh.edu \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).