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

Why? #26588

Merged
merged 1 commit into from
Jun 21, 2023
Merged

Why? #26588

merged 1 commit into from
Jun 21, 2023

Conversation

demeritcowboy
Copy link
Contributor

Overview

Why?

Before

Why?

After

I'm none-the-whyser, but there's less why.

Technical Details

Look at the next line.

Comments

@civibot
Copy link

civibot bot commented Jun 21, 2023

(Standard links)

@civibot civibot bot added the master label Jun 21, 2023
@demeritcowboy
Copy link
Contributor Author

jenkins retest this please

@larssandergreen
Copy link
Contributor

Maybe someone just needed to express how they feel about notes?

@colemanw colemanw merged commit 51e225d into civicrm:master Jun 21, 2023
@colemanw
Copy link
Member

I just spent an hour writing a detailed, 10 paragraph review of every facet of this PR, then immediately decided to overwrite it with this:
"Looks fine, merging."

@totten
Copy link
Member

totten commented Jun 21, 2023

But did you look at the review guidelines? The review criteria are:

  • Because
  • If
  • Somehow

@demeritcowboy
Copy link
Contributor Author

If anyone is curious, the overwrite line used to have an if around it, but it's been like this for 15 years.

@demeritcowboy demeritcowboy deleted the why branch June 21, 2023 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants