Skip to content

Add matplotlib to the benchmark extra's docstring/README package list - #279

Merged
yakew7 merged 2 commits into
yakew7:mainfrom
evanjain-dot:main
Aug 18, 2026
Merged

Add matplotlib to the benchmark extra's docstring/README package list#279
yakew7 merged 2 commits into
yakew7:mainfrom
evanjain-dot:main

Conversation

@evanjain-dot

Copy link
Copy Markdown
Contributor

pyproject.toml's benchmark extra gained matplotlib in #235 (figures.py
needs it to render strategy-comparison PNGs), but cli.py's module
docstring and README's install snippet still listed the old three
packages. Closes #244.

@evanjain-dot
evanjain-dot requested a review from yakew7 as a code owner August 18, 2026 15:18
@vercel

vercel Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

@evanjain-dot is attempting to deploy a commit to the yashkewlani2020-gmailcom's projects Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions

Copy link
Copy Markdown

@yakew7 - new PR touching a path you own, please review.

@yakew7

yakew7 commented Aug 18, 2026

Copy link
Copy Markdown
Owner

@ahmdkaml @Shreyash0712 if u want you can review it asw

@Shreyash0712

Copy link
Copy Markdown
Collaborator

Good to go. It's just adding matplotlib.

@Shreyash0712

Copy link
Copy Markdown
Collaborator

@yakew7 btw what perms do I have? can I merge stuff asw? Idts as I don't see the button to do so.

@yakew7

yakew7 commented Aug 18, 2026

Copy link
Copy Markdown
Owner

@Shreyash0712 you've got write access now (can push, open PRs, comment, review, etc.) and you're a co-code-owner of /explainers/ in CODEOWNERS - so on an explainers-only PR your approval satisfies the required-review gate and you'll see the merge button.

main is protected though: every PR needs 1 approving review, and it has to come from whoever CODEOWNERS says owns the touched paths (require_code_owner_reviews). this PR touches faircode/cli.py, which only I'm listed as owner for, so your approval alone doesn't unblock it - that's why the button isn't showing for you here, not a gap in your account.

on higher perms later - not ruling it out, not doing it yet either. you're the most prolific explainer contributor by far and the catch on #277 was a genuinely good piece of review, so it's earned you real trust. I'd want to see that extend past explainers first - reviewing/contributing to tests, scripts, or docs over the next couple months - before I'd extend CODEOWNERS further or anything closer to admin. will revisit then.

@yakew7
yakew7 merged commit 11533d0 into yakew7:main Aug 18, 2026
18 of 19 checks passed
@Shreyash0712

Copy link
Copy Markdown
Collaborator

I see. Love the system. A lot has changed in the repo since I joined. I'll do my best.

@yakew7

yakew7 commented Aug 18, 2026

Copy link
Copy Markdown
Owner

yeah the repo grew a lot more with some active contributors asw

@Shreyash0712

Copy link
Copy Markdown
Collaborator

Okay so can you make it so the bot starts to tag me in PRs as well? That way I'd get a notification.

@Shreyash0712

Copy link
Copy Markdown
Collaborator

Not just from my domain - all of them. I don't mind. I'll review whenever I get time. @yakew7

yakew7 added a commit that referenced this pull request Aug 18, 2026
Credits evanjain-dot's #277 (--html traceback fix + CLI test
coverage) and #279 (matplotlib docstring fix), propcgamer20-png's
#278 (favicon-parsing test coverage), and the two review-driven
follow-ups (Shreyash0712 on #277, ahmdkaml on #278). Also logs the
pr-review-ping.yml path-scoping fix, which wasn't recorded yet.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

faircode/cli.py's benchmark docstring omits matplotlib from the benchmark extra's package list

3 participants