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

fix typo #16865

Merged
merged 1 commit into from
Mar 20, 2020
Merged

fix typo #16865

merged 1 commit into from
Mar 20, 2020

Conversation

yashodha
Copy link
Contributor

fix typo

@civibot
Copy link

civibot bot commented Mar 20, 2020

(Standard links)

@civibot civibot bot added the master label Mar 20, 2020
@@ -524,7 +524,7 @@ public static function getPledgePaymentSearchFieldMetadata() {
}

/**
* Build the search for for pledges.
* Build the search for pledges.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I think it was supposed to read "Build the search form for pledges."

But it's a pretty useless comment if you ask me. The function is named CRM_Pledge_BAO_Query::buildSearchForm, so yea it's already quite obvious that it's meant to build a search form for pledges.

IMO if a comment doesn't add any useful information then it's just noise. What would be much more useful is a comment saying what type of form this is building and in what context this is used. Is this used to build an advanced search pane? A standalone search form? Both?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@colemanw the drupal standard is there should be a short sentence overview (beginning with a Capital & ending with a full stop) - regardless of whether it adds much info - but more info is better

@eileenmcnaughton eileenmcnaughton merged commit 3ae1634 into civicrm:master Mar 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants