On Thu, 31 Aug 2023, Alex Hung <alex.hung@xxxxxxx> wrote: > On 2023-08-30 01:29, Jani Nikula wrote: >> On Tue, 29 Aug 2023, Alex Hung <alex.hung@xxxxxxx> wrote: >>> There is a patch under internal reviews. It removes calls edid_override >>> and drm_edid_override_connector_update as intended in this patchset but >>> does not remove the functionality. >> >> While I am happy to hear there's progress, I'm somewhat baffled the >> review is internal. The commits that I suggested to revert were also >> only reviewed internally, as far as I can see... And that's kind of the >> problem. >> >> Upstream code should be reviewed in public. > > Hi Jani, > > All patches are sent for public reviews, the progress is summarized as > the followings: > > == internal == > > 1. a patch or patches are tested by CI. > 2. internal technical and IP reviews are performed to ensure no concerns > before patches are merged to internal branch. > > == public == > > 3. a regression test and IP reviews are performed by engineers before > sending to public mailing lists. > 4. the patchset is sent for public reviews ex. > https://patchwork.freedesktop.org/series/122498/ > 5. patches are merged to public repo. The point about public review is that there's no transparency to the steps before 4. The patches are posted for public review with Reviewed-by and Acked-by already added, based on internal review, and there is de facto no public review taking place on the code drops. There is zero visibility to the discussions taking place. We don't know if it's just rubber stamping, we don't know what concerns were raised, if any. I'm mainly disappointed about the double standards here, given that we post most patches directly upstream (especially ones that have zero reason to be embargoed like the ones being discussed here), and the ones that have gone through internal review will be stripped of all prior internal Reviewed-by's and Acked-by's before posting. Because that's the upstream expectation. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center
