All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nathan Chancellor <nathan@kernel.org>
To: Kortan <kortanzh@gmail.com>
Cc: ndesaulniers@google.com, clang-built-linux@googlegroups.com,
	linux-kernel@vger.kernel.org, llvm@lists.linux.dev
Subject: Re: [PATCH] fix missing 'sys' package
Date: Tue, 7 Sep 2021 13:06:54 -0700	[thread overview]
Message-ID: <YTfGXn4cTZ87zTP1@archlinux-ax161> (raw)
In-Reply-To: <20210907094336.16558-1-kortanzh@gmail.com>

Hi Kortan,

Thank you for the patch! I have some comments inline.

The commit title/subject should have a prefix to it. You can see the
prefix for this particular file by running:

$ git log --oneline --format=%s scripts/clang-tools/gen_compile_commands.py
gen_compile_commands: extract compiler command from a series of commands
gen_compile_commands: prune some directories
scripts/clang-tools: switch explicitly to Python 3
Makefile: Add clang-tidy and static analyzer support to makefile

So your commit title would be:

gen_compile_commands: fix missing 'sys' package

On Tue, Sep 07, 2021 at 05:43:36PM +0800, Kortan wrote:

There needs to be a message here. It is obvious once you look at the
file that we call sys.exit() so we need the import but that needs to be
explained up front here. checkpatch.pl would have warned you about this:

WARNING: Missing commit description - Add an appropriate one

total: 0 errors, 1 warnings, 7 lines checked

Otherwise, the change looks good to me. Please make these corrections
and send a v2 of the patch, which can be done with the '-v#' flag to
'git format-patch' (e.g. '-v2' in this case).

> Signed-off-by: Kortan <kortanzh@gmail.com>
> ---

Describe the diff between v1 and v2 here like:

v1 -> v2:

* Fix commit title

* Improve commit message

then be sure to also include Masahiro Yamada and the linux-kbuild
mailing list as he will pick up the patch.

Masahiro Yamada <masahiroy@kernel.org>
linux-kbuild@vger.kernel.org

Please use our new mailing list as well:

llvm@lists.linux.dev

Cheers,
Nathan

>  scripts/clang-tools/gen_compile_commands.py | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py
> index 0033eedce003..1d1bde1fd45e 100755
> --- a/scripts/clang-tools/gen_compile_commands.py
> +++ b/scripts/clang-tools/gen_compile_commands.py
> @@ -13,6 +13,7 @@ import logging
>  import os
>  import re
>  import subprocess
> +import sys
>  
>  _DEFAULT_OUTPUT = 'compile_commands.json'
>  _DEFAULT_LOG_LEVEL = 'WARNING'
> -- 
> 2.33.0
> 
> 

      reply	other threads:[~2021-09-07 20:06 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-07  9:43 [PATCH] fix missing 'sys' package Kortan
2021-09-07 20:06 ` Nathan Chancellor [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=YTfGXn4cTZ87zTP1@archlinux-ax161 \
    --to=nathan@kernel.org \
    --cc=clang-built-linux@googlegroups.com \
    --cc=kortanzh@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=ndesaulniers@google.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.