First of all, this is in my opinion very very ugly:
} else {
so If there was some vote or whatever, I would vote against it. I like vrata's version:
}
else {
more.
Second of all, I think if we are using the PEP 8 Python Coding Style, we could also use PEP 7 Python C Coding Style. It's already written, we have a lot of Python code, so we might as well use what they use for C too. So we don't need to "invent" and write up our own coding style. It's almost as reusing code. :)
It's hard to argue about what is right and what is wrong, when we don't have any written coding style, where we could look, and almost every file is written in different coding style.
----- Original Message -----
> On 11/15/2011 11:55 AM, Brian C. Lane wrote:
> > On Tue, Nov 15, 2011 at 12:58:08PM +0100, Vratislav Podzimek wrote:
> >> On Mon, 2011-11-14 at 09:48 -0800, Brian C. Lane wrote:
> >>> On Fri, Nov 11, 2011 at 01:38:14PM +0100, Vratislav Podzimek
> >>> wrote:
> >>>> Otherwise we have 'wifi activation failed' log message even if
> >>>> there's no wifi-related line in kickstart.
> >>>> ---
> >>>> loader/kickstart.c | 5 ++---
> >>>> 1 files changed, 2 insertions(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/loader/kickstart.c b/loader/kickstart.c
> >>>> index b038406..12082b1 100644
> >>>> --- a/loader/kickstart.c
> >>>> +++ b/loader/kickstart.c
> >>>> @@ -671,11 +671,10 @@ int process_kickstart_wifi (struct
> >>>> loaderData_s * loaderData) {
> >>>> WIFI_PROTECTION_UNPROTECTED, NULL,
> >>>> loaderData->ipinfo_set, loaderData->ipv4,
> >>>> loaderData->gateway, loaderData->dns,
> >>>> loaderData->netmask);
> >>>> }
> >>>> + if (rc == WIFI_ACTIVATION_OK) loaderData->netDev_set =
> >>>> 1;
> >>>> + else logMessage(ERROR, "wifi activation in kickstart
> >>>> failed");
> >>>
> >>> Please split the if/else into separate lines, it makes it more
> >>> readable.
> >> o.k., no problem, but have a look at
> >> http://www.redhat.com/archives/anaconda-devel-list/2011-May/msg00183.html
> >> where pjones argued against this style. Time to make an Anaconda
> >> Conding Style page?
> >>
> >
> > I don't think that's what he was complaining about. When dealing
> > with if
> > clauses that only have 1 statement in their block it should look
> > like
> > this:
> >
> > if (rc == WIFI_ACTIVATION_OK)
> > loaderData->netDev_set = 1;
> > else
> > logMessage(ERROR, "wifi activation in kickstart failed");
> >
> > I would also be happy with brackets, but only when they're on the
> > same
> > line, like this:
> >
> > if (rc == WIFI_ACTIVATION_OK) {
> > loaderData->netDev_set = 1;
> > } else {
> > logMessage(ERROR, "wifi activation in kickstart failed");
> > }
> >
> > If there is more than one statement in the block then brackets
> > should
> > look like the above. In your original patch you had some that had:
> >
> > }
> > else {
> >
> > Which (in my opinion of course), is ugly and a waste of space.
> >
> > function declarations should also have the { on the same line,
> > although
> > I see in our loader code we haven't always followed that style.
>
> Thanks for the clarification, Brian.
>
> Vratislav, I think a coding style page on the Anaconda wiki would be
> a
> good idea. We have a common style accepted by the team, but we do
> not
> have it documented anywhere. We also have a habit of breaking the
> style
> but only when we are trying to maintain consistency with existing
> code.
> Some things that should be explained on the coding style wiki page:
>
> 1) Items about if statements and other blocks, such as Brian
> explained
> above. We use 4 space indentation, and so on.
>
> 2) If your patch is against an older branch (e.g., rhel5-branch or
> rhel6-branch), avoid changing any coding style just to bring it up to
> date with our current coding style.
>
> 3) Separate coding style changes from functional changes in patches.
>
> 4) In general, coding style change patches should go on the master
> branch only. Once we branch for a Fedora or RHEL release, we should
> leave the style there alone.
>
> I'm sure we will have other things to add to the page, but I will
> wait
> for a first draft.
>
> --
> David Cantrell <dcantrell@xxxxxxxxxx>
> Supervisor, Installer Engineering Team
> Red Hat, Inc. | Westford, MA | EST5EDT
>
> _______________________________________________
> Anaconda-devel-list mailing list
> Anaconda-devel-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list
>
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
