How to reduce formal issues with new contributions from corporations?

[Paul Menzel]

Dear coreboot folks,

It’s great to see, that there a quite a few contributions – even the
majority – from people paid to do coreboot work.

Especially Intel and Google seem to employ a lot of developers working
on coreboot, and a lot of them push patches up for review. That’s
great.

My impression is though, that a lot of these contributions have formal
issues in the beginning. As the coding style and the commit message
guide lines are well documented in our Wiki [1][2], and there are even
scripts checking commits, the developers starting to work on coreboot
just don’t seem to know about this.

How can this be improved?

Could the companies make sure, that there developers read those, and
use the scripts?

Should this documentation be moved to the repository? A lot of the
“guideline Wiki pages” are locked anyway. People knowing the Linux
kernel, would expect that to also live in the source code repository?

Thanks,

Paul

[1] https://www.coreboot.org/Coding_Style
[2] https://www.coreboot.org/Git#Commit_messages

[Julius Werner]

I don't think it's helpful to single out "corporate developers" as the
black sheep that submit all the style violations. Every contributor, paid
or hobby, new or long-time, needs to pay attention to the project
guidelines equally. Everyone overlooks stuff some time, and it's normal
especially for new contributors to not be quite that familiar with
everything yet. Its our responsibility as reviewers (especially those of us
with +2 rights) to educate them and make sure the rules are followed before
a patch can get submitted.

That said, I do agree that we have a lot of potential in making this easier
and more consistent for everyone involved. I think the best tool for that
is more automated checking... like that recent idea to make Jenkins add the
checkpatch result visibly to its Verified+1 message.

I'm not sure if the "where" of the guidelines really makes much of a
difference... maybe we should just make them more discoverable either way?
Could we put a big orange "please ensure that this patch conforms to the
style guidelines at ..." banner in Gerrit somewhere near the submit button?

I also think that our guidelines could use a lot of cleaning up,
honestly... it's a big mess right now, and the harder they are to read the
easier it gets to accidentally or intentionally ignore them. Right now we
have https://www.coreboot.org/Development_Guidelines which includes a lot
of stuff duplicated from https://www.coreboot.org/Coding_Style. The latter
is almost a direct but not quite copy of the Linux Kernel Style Guide (to
the point of still including a bunch of stuff that makes no sense for
coreboot, like the net/ and drivers/net/ commenting exception), so it's
easy for people to take a quick look, think "oh, it's only a copy of this
thing I know already" and miss the subtle differences. I think it might
make more sense to cut that page and rewrite Development_Guidelines to say
"coreboot code must confirm to the Linux Kernel Style Guide <upstream link>
with the following exceptions/additions". And then we also have
Documentation/gerrit_guidelines.md in the source which is yet again a
different, slightly overlapping set of rules that should probably get
merged into the main guide.

[Stefan Reinauer]

* Paul Menzel via coreboot  [170226 14:37]:
> Dear coreboot folks,

[..]
> My impression is though, that a lot of these contributions have formal
> issues in the beginning. As the coding style and the commit message
> guide lines are well documented in our Wiki [1][2], and there are even
> scripts checking commits, the developers starting to work on coreboot
> just don’t seem to know about this.
>
> How can this be improved?

Hi Paul,

there is definitely a learning curve involved with people new to the
coreboot community. Thank you for pointing that out. Do you have a few
such examples of what you mean? That will make it much easier to address
these issues, than having to try and reach out to a large sub group of
coreboot community members with fairly vague feedback.
> Could the companies make sure, that there developers read those, and
> use the scripts?

Just like on the non-commercial side of the project, the people involved
in coreboot through their respective corporations are not necessarily a
single entity. I know of several independent groups at both Intel and
Google, that are contributing to coreboot. So while we might all share
the second half of our email addresses, that might be as far as the
similarities and overlaps go (and that's a great thing for coreboot).

Also, the list of contributors is continuously growing. Please be patient
with these folks, and continue to treat them like you would treat any
other individual in the coreboot community. I'm sure most of these folks
appreciate your mentorship on these issues.
> Should this documentation be moved to the repository? A lot of the
> “guideline Wiki pages” are locked anyway. People knowing the Linux
> kernel, would expect that to also live in the source code repository?

Yes, I am in favor of moving relevant documentation into the source
tree, as I believe that we should obsolete the wiki all together.

Stefan

On 02/27/2017 04:37 PM, Stefan Reinauer wrote:
> > Should this documentation be moved to the repository? A lot of the
> > “guideline Wiki pages” are locked anyway. People knowing the Linux
> > kernel, would expect that to also live in the source code repository?
>
> Yes, I am in favor of moving relevant documentation into the source
> tree, as I believe that we should obsolete the wiki all together.
>
> Stefan

Removing the wiki increases the learning curve for new users and would 
make it more difficult to find information at a glance.

I personally never would have started using coreboot if I had to view 
the source tree for documentation, that scares people away and a text 
file is not at all equivalent to a nicely formatted wiki page.