2023-12-14 02:16:17

by Brian Norris

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH v7 04/12] wifi: mwifiex: fixed missing WMM IE for assoc req.

On Fri, Dec 01, 2023 at 10:47:41PM +0000, David Lin wrote:
> > From: Francesco Dolcini <[email protected]>
> > Sent: Friday, December 1, 2023 6:19 PM

> > On Tue, Nov 28, 2023 at 04:31:07PM +0800, David Lin wrote:
> > > Remain on channel must be removed after authentication is done.
> > > Otherwise WMM setting for assoiation request will be removed.
> >
> > Same comment as patch 2, this seems a fixup of commit 1, you should fix
> > that patch, not add a followup fixup commit.
>
> So you think patch 1 to 4 should be merged as a single patch? In fact,
> patch 2 to 4 is issues reported by our QA for patch 1. If you insisted
> merge all of them, I can do this for patch v8.

In case you didn't get a sufficient answer elsewhere: yes, probably? We
don't care to see:

patch 1: introduce feature
patch 2: fix bug in patch 1
patch 3: fix bug in patch 1 and 2
patch 4: ...


Just ... actually fix patch 1, and send 1 patch. (Or more, if you have
several logical changes. Be sure to read [1].)

In case you're used to GitHub: we don't work like GitHub, where people
tend to stack a bunch of incremental changes during review, and then the
changes get squashed together before committing. We expect each patch to
be a good commit, and that it will get committed as-is.

If we're interested in the history and evolution of your changes, we can
look at the mailing list archives.

Brian

[1] https://docs.kernel.org/process/submitting-patches.html#separate-your-changes


2023-12-14 02:21:01

by David Lin

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH v7 04/12] wifi: mwifiex: fixed missing WMM IE for assoc req.

> From: Brian Norris <[email protected]>
> Sent: Thursday, December 14, 2023 10:16 AM
> To: David Lin <[email protected]>
> Cc: Francesco Dolcini <[email protected]>; [email protected];
> [email protected]; [email protected]; Pete Hsieh
> <[email protected]>
> Subject: Re: [EXT] Re: [PATCH v7 04/12] wifi: mwifiex: fixed missing WMM IE
> for assoc req.
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
>
>
> On Fri, Dec 01, 2023 at 10:47:41PM +0000, David Lin wrote:
> > > From: Francesco Dolcini <[email protected]>
> > > Sent: Friday, December 1, 2023 6:19 PM
>
> > > On Tue, Nov 28, 2023 at 04:31:07PM +0800, David Lin wrote:
> > > > Remain on channel must be removed after authentication is done.
> > > > Otherwise WMM setting for assoiation request will be removed.
> > >
> > > Same comment as patch 2, this seems a fixup of commit 1, you should
> > > fix that patch, not add a followup fixup commit.
> >
> > So you think patch 1 to 4 should be merged as a single patch? In fact,
> > patch 2 to 4 is issues reported by our QA for patch 1. If you insisted
> > merge all of them, I can do this for patch v8.
>
> In case you didn't get a sufficient answer elsewhere: yes, probably? We don't
> care to see:
>
> patch 1: introduce feature
> patch 2: fix bug in patch 1
> patch 3: fix bug in patch 1 and 2
> patch 4: ...
>
>
> Just ... actually fix patch 1, and send 1 patch. (Or more, if you have several
> logical changes. Be sure to read [1].)
>
> In case you're used to GitHub: we don't work like GitHub, where people tend
> to stack a bunch of incremental changes during review, and then the
> changes get squashed together before committing. We expect each patch to
> be a good commit, and that it will get committed as-is.
>
> If we're interested in the history and evolution of your changes, we can look
> at the mailing list archives.
>
> Brian

Thanks for your information. In fact Patch v8 is almost ready and it only includes two patches: one for client mode and one for AP mode.

David

>
> [1]
> https://docs.ke/
> rnel.org%2Fprocess%2Fsubmitting-patches.html%23separate-your-changes&
> data=05%7C02%7Cyu-hao.lin%40nxp.com%7Cc031726831234d8efdf108dbfc4
> aa160%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638381169686
> 467616%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2l
> uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=fg0y2
> dg6hCyUyHyDnBQS3PmMQqJD5n1h2lpq0ea9tys%3D&reserved=0