[libcamera-devel] [PATCH] readme: Convert tabs to spaces

Laurent Pinchart laurent.pinchart at ideasonboard.com
Tue Dec 28 09:00:28 CET 2021 

Hi Johann,

On Tue, Dec 28, 2021 at 11:51:40AM +0900, Johann Koenig wrote:
> Apologies for the mess! I'm trying to figure out the submission process
> so I wanted to send an easy patch to test.

No need to apologize :-)

> On Mon, Dec 27, 2021 at 10:18:14PM +0200, Laurent Pinchart wrote:
> > On Mon, Dec 27, 2021 at 10:56:41AM +0200, Laurent Pinchart wrote:
> > > On Mon, Dec 27, 2021 at 10:55:02AM +0200, Laurent Pinchart wrote:
> > > > On Mon, Dec 27, 2021 at 09:31:12AM +0100, Jacopo Mondi wrote:
> > > > > On Mon, Dec 27, 2021 at 01:33:43PM +0900, Johann Koenig via libcamera-devel wrote:
> > > > > > Date: Mon, 27 Dec 2021 13:33:43 +0900
> > > > > > From: Johann Koenig <johannkoenig at google.com>
> > > > > > To: libcamera-devel at lists.libcamera.org
> > > > > > X-Mailer: git-send-email 2.34.1.448.ga2b2bfdf31-goog
> > > > > > Subject: [PATCH] readme: Convert tabs to spaces
> > > > > 
> > > > > Weird, this shouldn't be here. If I do apply the patch these line show
> > > > > in the commit history
> > > > > 
> > > > > $ git show
> > > > > commit 833806e65611da1bbffae9aaa807d5b9dc5183f5 (HEAD)
> > > > > Author: Johann Koenig <johannkoenig at google.com>
> > > > > Date:   Mon Dec 27 13:33:43 2021 +0900
> > > > > 
> > > > >     readme: Convert tabs to spaces
> > > > > 
> > > > >     To: libcamera-devel at lists.libcamera.org
> > > > >     X-Mailer: git-send-email 2.34.1.448.ga2b2bfdf31-goog
> > > > >     Subject: [PATCH] readme: Convert tabs to spaces
> > > > > 
> > > > >     The initial commit (a146cdb) used tabs for all the dependencies. The
> > > > >     first time a line was edited (2de7843/30f9624) it did not keep the
> > > > >     tab. This makes them all consistent.
> > > > > 
> > > > >     Signed-off-by: johannkoenig at google.com
> > 
> > Another comment, this should be
> > 
> > Signed-off-by: Johann Koenig <johannkoenig at google.com>
> > 
> > git config --global user.name "Johann Koenig"
> > 
> > will help for future patches.
> > 
> > I'll also fix it when pushing the patch.
> 
> Thanks! I had manually written that. Just enabled
> .git/hooks/prepare-commit-msg to automate it.

You can also just use `git commit -s`.

> > > > > I wonder how did it happened since it seems you've used git send-email
> > > > > and most probably generated the patch with git format-patch.
> 
> Even simpler: git send-email HEAD~1.
> 
> > > > Welcome to DMARC. The google.com domain uses a strict DMARC policy that
> > > > requires recipients to ignore e-mails that fail DKIM and SPF tests. This
> > > > breaks mailing lists, so mailman has to wrap the original message in an
> > > > RFC822 payload. It looks like `git am` has trouble coping. I don't
> > > > recall seeing this problem before, so I wonder if it could be a
> > > > regression in git, but I may simply not have applied affected patches
> > > > from e-mail.
> > > > 
> > > > Note that the chromium.org domain isn't affected.
> 
> Thanks for the explanation. I'll look into it a little further and if I
> don't see a way to fix it I can switch to @chromium.org.

I don't think there's a solution with @google.com. It's not a huge deal
though, but it reminds me I need to update my patch apply scripts to
support this properly (or fix `git am`).

> > > > > As a suggestion: we usually try to refer to commit using the
> > > > > canonical 12-digits format. In example:
> > > > > 
> > > > > The initial commit a146cdbf20cb ("readme: Provide build requirements")
> > > > > used tabs for all the dependencies. The first time a line was edited
> > > > > in commit 2de78434ca71 ("meson: Bump required version to 0.47") tabs
> > > > > were not kept. This makes them all consistent.
> 
> Thanks! Will keep that in mind for the next time.
> 
> > > > > > The initial commit (a146cdb) used tabs for all the dependencies. The
> > > > > > first time a line was edited (2de7843/30f9624) it did not keep the
> > > > > > tab. This makes them all consistent.
> > > > > 
> > > > > This makes me wonder if we shouldn't have gone for all tabs then ?
> > > > > Anyway, consistency is what matters, so whatever is fine
> > > > 
> > > > https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#whitespace
> > > > 
> > > > "Spaces are recommended for indentation, but tabs may also be used. Tabs
> > > > will be converted to spaces. Tab stops are at every 8th column
> > > > (processing systems may make this value configurable)."
> > > > 
> > > > Spaces sound good then.
> 
> I only noticed this because my editor is set to highlight tabs in red.
> I'm happy to use tabs if they are preferred or predominant.
> 
> > > > > Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> > > > 
> > > > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > 
> > > No need to resend by the way, I'll update the commit message when
> > > applying.
> 
> Thanks!
> 
> > > > > > Signed-off-by: johannkoenig at google.com
> > > > > > ---
> > > > > >  README.rst | 10 +++++-----
> > > > > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > > > >
> > > > > > diff --git a/README.rst b/README.rst
> > > > > > index c48b4dba..05f0e6f2 100644
> > > > > > --- a/README.rst
> > > > > > +++ b/README.rst
> > > > > > @@ -44,7 +44,7 @@ The following Debian/Ubuntu packages are required for building libcamera.
> > > > > >  Other distributions may have differing package names:
> > > > > >
> > > > > >  A C++ toolchain: [required]
> > > > > > -	Either {g++, clang}
> > > > > > +        Either {g++, clang}
> > > > > >
> > > > > >  Meson Build system: [required]
> > > > > >          meson (>= 0.55) ninja-build pkg-config
> > > > > > @@ -78,19 +78,19 @@ for the Raspberry Pi IPA: [optional]
> > > > > >           'pipelines' option to avoid this dependency.
> > > > > >
> > > > > >  for device hotplug enumeration: [optional]
> > > > > > -	libudev-dev
> > > > > > +        libudev-dev
> > > > > >
> > > > > >  for documentation: [optional]
> > > > > > -	python3-sphinx doxygen graphviz texlive-latex-extra
> > > > > > +        python3-sphinx doxygen graphviz texlive-latex-extra
> > > > > >
> > > > > >  for gstreamer: [optional]
> > > > > > -	libgstreamer1.0-dev libgstreamer-plugins-base1.0-dev
> > > > > > +        libgstreamer1.0-dev libgstreamer-plugins-base1.0-dev
> > > > > >
> > > > > >  for cam: [optional]
> > > > > >          libevent-dev
> > > > > >
> > > > > >  for qcam: [optional]
> > > > > > -	qtbase5-dev libqt5core5a libqt5gui5 libqt5widgets5 qttools5-dev-tools libtiff-dev
> > > > > > +        qtbase5-dev libqt5core5a libqt5gui5 libqt5widgets5 qttools5-dev-tools libtiff-dev
> > > > > >
> > > > > >  for tracing with lttng: [optional]
> > > > > >          liblttng-ust-dev python3-jinja2 lttng-tools

-- 
Regards,

Laurent Pinchart

More information about the libcamera-devel mailing list