Search Linux Wireless

Re: [PATCH v7 00/12] wifi: mwifiex: added code to support host mlme.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Lin,
thanks for the patches here, I can clearly see that this code is going
through some real testing given the improvements you did lately.

I have commented on the single patches, and honestly I did not look into
the code details at the moment.

The major feedback from me is the following:
 1 - you should not add code with a bug and than fix a bug in the same
     series, you should have a non buggy patch in the first place (e.g.
     git --amend). (this applies till the patch is not merged into the
     maintainer tree, of course).
 2 - point 1 applies also to reviewer comments
 3 - if you have fixes that are not connected to the feature addition
     you are doing is beneficial to have those separated, this makes
     reviewing easier, they can be "prioritized" to some extent (given
     that they are fixes) and follow a slightly different patch flow
     (they can get applied, depending on the maintainers decision, when the
     merge window is closed and should be backported). Not to mention
     that smaller patch series are appreciated, "Maximum of 7-12 patches
     per patchset " from [1]

In general I would suggest you to have a look at [1], not sure how up to
date is that compared to the in-tree Documentation/process/.

On Tue, Nov 28, 2023 at 04:31:03PM +0800, David Lin wrote:
> 5. Address reviewer comments.
You should list the changes you did, something that generic is forcing
the reviewer to compare v7 vs v6 to known what changed.


[1] https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Francesco





[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux