| Message ID | 20220723205319.3326374-1-jan@3e8.eu |
|---|---|
| Headers | show |
| Series | realtek: add HPE 1920 support | expand |
> This adds support for three switches from the HPE 1920 series. It has > been tested on HPE 1920-8G and HPE 1920-16G. Support for HPE 1920-24G > is also included, as it uses the same board as the 16-port model. > > The patch series depends on the firmware-utils patch adding the > mkh3cimg and mkh3cvfs tools. This patch series has just been merged. However, as mentioned in the cover letter, it depends on new tools in firmware-utils. As the corresponding patch series ("add tools for H3C devices") hasn't been merged yet, this won't actually build. Please tell me if I should have made this dependency clearer in any way, so I can avoid similar situations in the future. Jan
On Thu, 2022-07-28 at 15:10 +0200, Jan Hoffmann wrote: > > This adds support for three switches from the HPE 1920 series. It has > > been tested on HPE 1920-8G and HPE 1920-16G. Support for HPE 1920-24G > > is also included, as it uses the same board as the 16-port model. > > > > The patch series depends on the firmware-utils patch adding the > > mkh3cimg and mkh3cvfs tools. > > This patch series has just been merged. However, as mentioned in the > cover letter, it depends on new tools in firmware-utils. As the > corresponding patch series ("add tools for H3C devices") hasn't been > merged yet, this won't actually build. I've pushed a revert commit to avoid breaking the builds. Can someone explain why this whole series needed to be merged so quickly? These *7* patches have only been posted on Saturday, and testing was performed for patches 2-4. At least patches 5-7 could've remained unmerged awaiting some further comments. > > Please tell me if I should have made this dependency clearer in any way, > so I can avoid similar situations in the future. It was plainly stated in the cover letter, I don't think you could have been any clearer about this dependency. Best, Sander
On Thu, Jul 28, 2022 at 03:27:14PM +0200, Sander Vanheule wrote: > On Thu, 2022-07-28 at 15:10 +0200, Jan Hoffmann wrote: > > > This adds support for three switches from the HPE 1920 series. It has > > > been tested on HPE 1920-8G and HPE 1920-16G. Support for HPE 1920-24G > > > is also included, as it uses the same board as the 16-port model. > > > > > > The patch series depends on the firmware-utils patch adding the > > > mkh3cimg and mkh3cvfs tools. > > > > This patch series has just been merged. However, as mentioned in the > > cover letter, it depends on new tools in firmware-utils. As the > > corresponding patch series ("add tools for H3C devices") hasn't been > > merged yet, this won't actually build. > > I've pushed a revert commit to avoid breaking the builds. > > Can someone explain why this whole series needed to be merged so quickly? These *7* patches have > only been posted on Saturday, and testing was performed for patches 2-4. At least patches 5-7 > could've remained unmerged awaiting some further comments. As 5-7 depend on the actual device for testing they are unlikely to receive great amounts of feedback, but are also unlikely to break anything. > > > > > Please tell me if I should have made this dependency clearer in any way, > > so I can avoid similar situations in the future. > > It was plainly stated in the cover letter, I don't think you could have been any clearer about this > dependency. Yes, it was absolutely clear and I had it in my local firmware-utils tree for which I used CONFIG_SRC_TREE_OVERRIDE. The fact that the cover letter doesn't become part of git history was part of the reason why I forgot about that before pushing.
On Thu, 2022-07-28 at 16:51 +0200, Daniel Golle wrote: > On Thu, Jul 28, 2022 at 03:27:14PM +0200, Sander Vanheule wrote: > > On Thu, 2022-07-28 at 15:10 +0200, Jan Hoffmann wrote: > > > > This adds support for three switches from the HPE 1920 series. It has > > > > been tested on HPE 1920-8G and HPE 1920-16G. Support for HPE 1920-24G > > > > is also included, as it uses the same board as the 16-port model. > > > > > > > > The patch series depends on the firmware-utils patch adding the > > > > mkh3cimg and mkh3cvfs tools. > > > > > > This patch series has just been merged. However, as mentioned in the > > > cover letter, it depends on new tools in firmware-utils. As the > > > corresponding patch series ("add tools for H3C devices") hasn't been > > > merged yet, this won't actually build. > > > > I've pushed a revert commit to avoid breaking the builds. > > > > Can someone explain why this whole series needed to be merged so quickly? These *7* patches have > > only been posted on Saturday, and testing was performed for patches 2-4. At least patches 5-7 > > could've remained unmerged awaiting some further comments. > > As 5-7 depend on the actual device for testing they are unlikely to > receive great amounts of feedback, but are also unlikely to break > anything. Well, I started with replying to the firmware-utils changes yesterday. Now those two patches are merged with still a plethora of errors and warnings from checkpatch.pl, and someone will have to clean up those files incrementally. In general, I feel that if anyone thinks patches are good enough without any real discussion, or is waiting for changes to be merged, I would strongly prefer that they express this via a public notice on those patches. At least give people 24h to let you know they still want to review something. Best, Sander > > > > > > > > > Please tell me if I should have made this dependency clearer in any way, > > > so I can avoid similar situations in the future. > > > > It was plainly stated in the cover letter, I don't think you could have been any clearer about > > this > > dependency. > > Yes, it was absolutely clear and I had it in my local firmware-utils > tree for which I used CONFIG_SRC_TREE_OVERRIDE. > The fact that the cover letter doesn't become part of git history was > part of the reason why I forgot about that before pushing. > >