Ksummit-Discuss Archive on lore.kernel.org
 help / color / Atom feed
From: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: ksummit-discuss@lists.linuxfoundation.org,
	Linux Media Mailing List <linux-media@vger.kernel.org>
Subject: Re: [Ksummit-discuss] [PATCH] media: add a subsystem profile documentation
Date: Wed, 18 Sep 2019 15:48:31 -0300
Message-ID: <20190918154831.3dd0d040@coco.lan> (raw)
In-Reply-To: <20190918172705.GC4734@pendragon.ideasonboard.com>

Em Wed, 18 Sep 2019 20:27:05 +0300
Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:

> > Anyway, not sure if the other sub-maintainers see the same way. From my side,
> > I prefer not to be c/c, as this is just more noise, as I just rely on
> > patchwork for media patches. What about changing this to:
> > 
> > 	Patches for the media subsystem should be sent to the media mailing list
> > 	at linux-media@vger.kernel.org as plain text only e-mail. Emails with
> > 	HTML will be automatically rejected by the mail server. It could be wise 
> > 	to also copy the sub-maintainer(s).  
> 
> That works for me. As this is really a personal preference, is there a
> way it could be encoded in MAINTAINERS in a per-person fashion ?
> Something that would allow you to opt-out from CC from linux-media (but
> possibly opt-in for other parts of the kernel), and allow me to opt-in
> for the drivers I maintain ?

I don't think so. Perhaps we could add, instead, something like that at the
sub-maintainers section of the profile.

> > > > +Submit Checklist Addendum
> > > > +-------------------------
> > > > +
> > > > +There is a set of compliance tools at https://git.linuxtv.org/v4l-utils.git/
> > > > +that should be used in order to check if the drivers are properly
> > > > +implementing the media APIs.
> > > > +
> > > > +Those tests need to be passed before the patches go upstream.    
> > > 
> > > s/need to be passed/need to pass/
> > >   
> > > > +
> > > > +Also, please notice that we build the Kernel with::
> > > > +
> > > > +	make CF=-D__CHECK_ENDIAN__ CONFIG_DEBUG_SECTION_MISMATCH=y C=1 W=1 CHECK=check_script
> > > > +
> > > > +Where the check script is::
> > > > +
> > > > +	#!/bin/bash
> > > > +	/devel/smatch/smatch -p=kernel $@ >&2
> > > > +	/devel/sparse/sparse $@ >&2
> > > > +
> > > > +Be sure to not introduce new warnings on your patches.    
> > > 
> > > While static analysers deliver value, I fear this is a high barrier to
> > > entry for new contributors.  
> > 
> > Will change this to:
> > 
> > 	Be sure to not introduce new warnings on your patches without a 
> > 	very good reason.
> > 
> > Especially for new contributors, I really expect patches to not introduce
> > new warnings, as it is usually a lot more painful to fix them later than
> > to ask devs to do things right at the first place.  
> 
> I fully agree with the goal, but asking a drive-by contributor, who
> usually has to go through issues with sending patches through e-mail
> already, to install smatch and sparse and use them, is setting the bar
> high. I think automating those checks is the way to go.

Yeah, I have plans to automate it, the same way we did for pull
requests. I'm planning to do that later, after upgrading patchwork
to the current upstream version (with requires upgrading some libraries
too at the server).

In any case, having this at the profile is a reminder for whomever is 
submitting a patch that it should be clean for static analyzers too.

> > > > +Coding Style Addendum
> > > > +---------------------
> > > > +
> > > > +Media development uses checkpatch on strict mode to verify the code style, e. g.::
> > > > +
> > > > +	$ ./script/checkpatch.pl --strict    
> > > 
> > > But we still accept some warnings. I think this should be documented.  
> > 
> > True, but not sure if we should enter into too specific details here.
> > 
> > What about adding something like:
> > 
> > 	Please notice that the goal here is to improve code readability. On a few 
> > 	cases, checkpatch may actually point to	something that would look worse. 
> > 
> > 	So, you	should use good	send sense here, being prepared to justify any   
> 
> s/send sense/sense/

Gah... where this "send" came from??? :-p

> 
> > 	coding style decision.  
> 
> "being prepared to justify" sounds a bit harsh :-) But the general
> message is good.

Any suggestions for a lighter text with similar meaning? :-)

> > 	Please also notice that, on some cases,	when you fix one issue,	you may
> > 	receive	warnings about lines longer than 80 columns. It	is fine	to have
> > 	longer lines if	this means that	other warnings will be fixed by	that.
> > 
> > 	Yet, if	you're having more than	80 columns on a	line, please consider 
> > 	simplifying the	code - if too idented -	or to use shorter names	for 
> > 	variables.  
> 
> That's already quite specific for my taste. We can keep it as-is if you
> think it's fine, but we clearly shouldn't go into more details.

Yeah, I see. Yet, IMHO, we should either not add anything about checkpatch
warnings that might not be relevant or add something similar like the above.

Thanks,
Mauro
_______________________________________________
Ksummit-discuss mailing list
Ksummit-discuss@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss

  reply index

Thread overview: 82+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-11 15:48 [Ksummit-discuss] [PATCH v2 0/3] Maintainer Entry Profiles Dan Williams
2019-09-11 15:48 ` [Ksummit-discuss] [PATCH v2 1/3] MAINTAINERS: Reclaim the P: tag for Maintainer Entry Profile Dan Williams
2019-09-13 15:37   ` Mauro Carvalho Chehab
2019-09-11 15:48 ` [Ksummit-discuss] [PATCH v2 2/3] Maintainer Handbook: " Dan Williams
2019-09-16 12:35   ` Jani Nikula
2019-10-01 13:55   ` Jonathan Corbet
2019-10-01 18:17     ` Martin K. Petersen
2019-11-07 20:13     ` Jonathan Corbet
2019-11-08  2:41       ` Dan Williams
2019-09-11 15:48 ` [Ksummit-discuss] [PATCH v2 3/3] libnvdimm, MAINTAINERS: " Dan Williams
2019-09-11 17:42   ` Vishal Verma
2019-09-11 18:43   ` Dan Carpenter
2019-09-11 22:11     ` Jens Axboe
2019-09-12  7:41       ` Dan Williams
     [not found]         ` <CANiq72k2so3ZcqA3iRziGY=Shd_B1=qGoXXROeAF7Y3+pDmqyA@mail.gmail.com>
2019-09-12 10:18           ` Joe Perches
2019-09-13  7:09       ` Jonathan Corbet
2019-09-13 11:48         ` Dan Carpenter
2019-09-13 12:18           ` Dan Williams
2019-09-13 15:00           ` Randy Dunlap
2019-09-13 15:46             ` Rob Herring
2019-09-13 16:42               ` Joe Perches
2019-09-13 19:32                 ` Rob Herring
2019-09-13 17:57             ` Geert Uytterhoeven
2019-09-16 12:42           ` Jani Nikula
     [not found]           ` <20190917161608.GA12866@ziepe.ca>
2019-09-17 21:59             ` Dan Williams
2019-09-13 21:44       ` Martin K. Petersen
2019-09-16  7:01         ` Dan Carpenter
2019-09-16 17:08           ` Martin K. Petersen
2019-09-16 17:15             ` Mark Brown
2019-09-11 20:30   ` Joe Perches
2019-09-13 16:19   ` [Ksummit-discuss] [PATCH] media: add a subsystem profile documentation Mauro Carvalho Chehab
2019-09-13 16:19     ` Mauro Carvalho Chehab
2019-09-17  3:35     ` [Ksummit-discuss] single maintainer profile directory (was Re: [PATCH] media: add a subsystem profile documentation) Kees Cook
2019-09-17 13:28       ` Mauro Carvalho Chehab
2019-09-17 16:33         ` Kees Cook
2019-09-18 11:23           ` Mauro Carvalho Chehab
2019-09-18 17:39             ` Kees Cook
2019-09-18 18:35               ` Mauro Carvalho Chehab
2019-09-21 19:13             ` Jonathan Corbet
2019-09-21 19:45               ` Mauro Carvalho Chehab
2019-09-23 22:45               ` Kees Cook
2019-09-18 12:36     ` [Ksummit-discuss] [PATCH] media: add a subsystem profile documentation Laurent Pinchart
2019-09-18 13:57       ` Mauro Carvalho Chehab
2019-09-18 17:27         ` Laurent Pinchart
2019-09-18 18:48           ` Mauro Carvalho Chehab [this message]
2019-09-19  7:08             ` Dan Carpenter
2019-09-20  5:29               ` Joe Perches
2019-09-20 14:09                 ` Daniel Vetter
2019-09-19  6:56         ` Dan Carpenter
2019-09-19  7:22           ` Geert Uytterhoeven
2019-09-19  8:49             ` Jani Nikula
2019-09-19  8:58               ` Geert Uytterhoeven
2019-09-19  9:52                 ` Jani Nikula
2019-09-20 14:53             ` Laurent Pinchart
2019-09-20 14:59               ` Doug Anderson
2019-09-21  8:56                 ` Jani Nikula
2019-09-23 15:58                   ` Doug Anderson
2019-09-23 16:04                     ` Jonathan Corbet
2019-09-19  9:52           ` Mauro Carvalho Chehab
2019-09-25 17:13           ` Joe Perches
2019-09-25 18:40             ` Kees Cook
2019-09-26 15:14               ` Joe Perches
2019-09-26 15:53                 ` Kees Cook
2019-09-26 16:02                   ` Joe Perches
2019-09-26 16:24                     ` Kees Cook
2019-09-26 10:25             ` Geert Uytterhoeven
2019-09-18 13:59       ` [Ksummit-discuss] [PATCH v2] " Mauro Carvalho Chehab
     [not found]         ` <1e479f17-dbc8-b44d-bd1e-4229a6dbf151@collabora.com>
2019-09-18 14:11           ` Mauro Carvalho Chehab
2019-09-11 16:40 ` [Ksummit-discuss] [PATCH v2 0/3] Maintainer Entry Profiles Martin K. Petersen
2019-09-12 13:31   ` Bart Van Assche
2019-09-12 15:34     ` Joe Perches
2019-09-12 20:01       ` Bart Van Assche
2019-09-12 20:34         ` Joe Perches
2019-09-13 14:26           ` Rob Herring
2019-09-13 18:42             ` Joe Perches
2019-09-13 19:17               ` Mauro Carvalho Chehab
2019-09-13 20:33                 ` Joe Perches
2019-09-13 12:56         ` Matthew Wilcox
2019-09-13 13:54           ` Mauro Carvalho Chehab
2019-09-13 14:59             ` Guenter Roeck
2019-09-13 22:03     ` Martin K. Petersen
2019-09-12 13:10 ` Bart Van Assche

Reply instructions:

You may reply publically 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=20190918154831.3dd0d040@coco.lan \
    --to=mchehab+samsung@kernel.org \
    --cc=ksummit-discuss@lists.linuxfoundation.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    /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

Ksummit-Discuss Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/ksummit-discuss/0 ksummit-discuss/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ksummit-discuss ksummit-discuss/ https://lore.kernel.org/ksummit-discuss \
		ksummit-discuss@lists.linuxfoundation.org
	public-inbox-index ksummit-discuss

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.linuxfoundation.lists.ksummit-discuss


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git