[staging-next] staging/mei: mei-amt-version - make all function static

Dan Carpenter dan.carpenter at oracle.com
Mon Feb 20 11:29:11 UTC 2012


On Mon, Feb 20, 2012 at 11:12:58AM +0000, Winkler, Tomas wrote:
> > 
> > On Mon, Feb 20, 2012 at 11:54:46AM +0200, Tomas Winkler wrote:
> > > since this is a single file example make also API-like function static
> > > and used to remove compilation warnings.
> > >
> > > Signed-off-by: Tomas Winkler <tomas.winkler at intel.com>
> > 
> > [ snip ]
> > 
> > > @@ -455,6 +455,8 @@ int main(int argc, char **argv)
> > >
> > >  	status = amt_get_code_versions(&acmd, &ver);
> > >
> > > +	amt_host_if_deinit(&acmd);
> > > +
> > 
> > The changelog sucks.  What is the effect of this change?  Why don't we just
> > delete the function instead?
> 
> It is an  API example so I don't want to remove this function
> I believe it show the usage  in proper place, yes it also eliminates
> A compilation warning. 
> 
> Maybe the proper way would be to add API declaration section but
> I'm not sure it'll add anything more descriptive. 
> 

The subject says "make all function static" and the changelog says
it's to "remove compile warnings".  So I was thinking, "Hm.
Probably he meant Sparse warnings, not compile warnings."  But then
I saw the patch introduced a new function call.  And I'm thinking
wtf?

One thing that would help and which is a standard is that when you
are fixing compile warnings, put a copy of the warning into the
changelog.

I don't like to complain about grammar and spelling issues in
changelogs, because it's a second language for so many people.  But
your English in emails is very good with proper capitalization and
everything.  Could you try writing better changelogs?

regards,
dan carpenter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/attachments/20120220/5cb67b5d/attachment.asc>


More information about the devel mailing list