Re: [dts] [PATCH v2] ci/initial: Added script to get the tests for a patchset

["Juraj Linkeš" <juraj.linkes@pantheon.tech>]

> -----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
>