Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify regex in yelling.py #164

Closed
wants to merge 1 commit into from
Closed

Simplify regex in yelling.py #164

wants to merge 1 commit into from

Conversation

bradleysigma
Copy link
Contributor

Noticed the superfluous regex in https://www.youtube.com/live/6wFATYjMTQE?t=1376

@andrewj-brown
Copy link
Member

andrewj-brown commented Oct 2, 2023

This is honestly a little disappointing. You're a regular contributor. We have 18 open issues you could have put work into, and you yourself have an open PR needing changes (granted, unlikely to be merged anyway, for committee reasons, but still). Why are you wasting my review time and your coding time with a 3-character change? You're perfectly capable of, y'know, fulfilling the actual purpose of hacktoberfest and making positive contributions to the bot.

Hell, issue #160 is right up your alley, considering how much work you've put into the yelling cog. Why wouldn't you go do that?

If you want a concrete reason why I'm not merging this...

  • it's polluting the git blame, and making it unclear who's fault this regex is (not that it's a particularly complicated regex, but on principle, git blame should always be useful & accurate)
  • it's using more-specific regex syntax, which actually makes it less easily readable than using the language-agnostic [^\s] construct - we're not code golfing here
  • it's sending me stupid emails and interrupting me on a day when I need to be doing actual work on my assignments
  • and, it's sending a poor message to external or future contributors that we're rubber-stamping hacktoberfest bullshit PRs from people in the "in group", making it harder to argue the case that we shouldn't accept them next time someone else submits one.

@JamesDearlove JamesDearlove added invalid This doesn't seem right and removed hacktoberfest-accepted labels Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants