Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1292374 > unrolled thread

RE: [PATCH 21/40] staging: lustre: improve LNet clean up code and API

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  RE: [PATCH 21/40] staging: lustre: improve LNet clean up code and API "Simmons, James A." <simmonsja@ornl.gov> - 2015-12-15 18:30 +0100
    Re: [PATCH 21/40] staging: lustre: improve LNet clean up code and API Dan Carpenter <dan.carpenter@oracle.com> - 2015-12-15 18:50 +0100

#1292374 — RE: [PATCH 21/40] staging: lustre: improve LNet clean up code and API

>Actually we're going to have to redo so much code that it's not worth it
>for me to review the rest of these patches.  

Sorry I didn't get back to you sooner but I was on vacation.  Thanks for 
reviewing this work. Especially since this is the first major bug fixing merge
for the lustre client which means a lot of pain involved to iron out how to
do this. I have been pondering if pushing bug fixes before style cleanups
is the right thing to do. I pushed a bunch of bug fixes earlier and none got
merged which either means Greg is just backed up and hasn't the time to
merge them or  style issues are higher priority. Assuming these bug fixes are
in scope of the staging tree. Should I continue to push this work first?
Well either way I should update this patch series so it ready to merge at some
point.

>Please just look over everything again:
>
> BAD:	return -1;
>GOOD:	return -EINVAL;
>
> BAD:  failed0:
>GOOD:  free_something:
>
> BAD:	if (rc != 0)
>GOOD:	if (rc)
>
>Do one thing per patch.
>Do not introduce a bug and then fix it in a later patch.
>Check ioc_len more carefully.
>Don't make the code look ugly just to please checkpatch.pl.
>Do error handling not success handling.
>Try to avoid indenting a far to the right.

Okay. Will start to do the patch cleanup.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1292405

On Tue, Dec 15, 2015 at 05:10:39PM +0000, Simmons, James A. wrote:
>I have been pondering if pushing bug fixes before style cleanups
> is the right thing to do.

Generally push the least controversial patches first so that if you
have to redo one patch, then the rest are already applied and don't need
to be changed.

> I pushed a bunch of bug fixes earlier and none got
> merged which either means Greg is just backed up and hasn't the time to
> merge them or  style issues are higher priority

I have no idea which patchset you are talking about so I can't comment.
Greg always (except if there is a mistake) applies things in first come
first serve order.  He doesn't sort them.

> Assuming these bug fixes are in scope of the staging tree. Should I
> continue to push this work first?

You've pushed a bunch of stuff.  I don't know which stuff has been
applied and which has not.  If no one replied to it and there isn't a
dire issue such as a compile failure or it doesn't apply then Greg is
likely to apply it.  He doesn't silently patches, so you will get an
email either way.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web