new code pushed to main branch?

[illume]

It looks like a big commit was just pushed to the main branch without a PR.

Seems it's a bunch of commits together from a fork somewhere?

[jaitaiwan]

What do you mean by big commit? It's 3 files. Or bunch of commits it's a single commit? Maintainers will often push directly to the branch ourselves where required. You see the same thing on many other gorilla repos. Eventually there will be a release which will include this change and whatever others we merge or push.

[illume]

I wondered about a commit pushed to main by a new contributor is all. I wrongly assumed code review would be done via PR because I saw an OpenSSF best practices badge.

cc @coreydaley

[coreydaley]

Only because I was tagged by @illume I will weigh in.
All contributed code SHOULD be a pull request so that it can be reviewed by the community, unless it fixes an embargoed CVE. I am not up-to-date on the circumstances surrounding this commit.

Since this user (@patrickod) is also a first time contributor it does seem a bit weird and suspicious. Maybe that user can weigh in on the circumstances surrounding this contribution?

[illume]

Since the commit wasn't reviewed in a PR, would you accept a PR reverting it?

[jaitaiwan]

Can you refer me to where in the best practices it is a MUST to have a PR for the review? I struggled to find anywhere that would enforce it

Considering the team has not yet done a release around this commit yet, I think you can pretty easily determine the why for this commit.

[illume]

Like mentioned above, all code should be contributed by pull request…

https://github.com/gorilla/.github/blob/ccd01604dd05f1f9ba58bf6c5cad19b3fb0cfaa4/CONTRIBUTING.md#all-code-changes-happen-through-pull-requests

[jaitaiwan]

Okay so you're only going off the contributing guide got it. So let me think through this for you... if the contributing guide says "commit a PR" and there's no visible PR that a commit has come from... then it must be.... an embargoed CVE 🥇

Therefore if it's embargoed the maintainers can't talk about it.

Honestly this is just about the silliest thing I've had the pleasure of dealing with in Open Source so far. There's a thorough explanation of the commit in the commit message and it's 3 files so it's not like it's hard to read it yourself to deduce this. Nor is it any less transparent than if it had gone through a public PR process.