We currently have a bit of a mess regarding class names of the Views plugins, mostly the filters: most of them start with SearchApiFilter, but some only with SearchApi, and while both has its downsides and both can be said to violate coding standards, we should at least be consistent. In most other cases, where we have more than one plugin of the type, we omit the "Filter" (or whatever) part, so I would do the same here.
I.e., just rename all classes that start with SearchApiFilter and remove the Filter from the name (not the trait, though!).

In the same spirit, SearchApiArgument should probably be renamed to SearchApiStandard or something like that. But do our arguments currently even work? Anyways, we can rename it in any case.

CommentFileSizeAuthor
#8 cleanup-class-names-2650124-8.patch10.28 KBrashid_786
#4 cleanup-class-names-2650124-4.patch12.55 KBrashid_786
#2 cleanup-class-names-2650124-2.patch44.69 KBrashid_786
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

drunken monkey created an issue. See original summary.

rashid_786’s picture

rashid_786 CreditAttribution: rashid_786 at SDG Corporation commented
Status: Active » Needs review
FileSize
cleanup-class-names-2650124-2.patch44.69 KB

Changes have been made in this patch as suggested, Kindly review and let me know if any changes required.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ for Acquia commented
Status: Needs review » Needs work

Can you recreate the patch with git diff -M? That makes reviewing a lot easier.
I think the @file docblocks need to be updated as well.

rashid_786’s picture

rashid_786 CreditAttribution: rashid_786 at SDG Corporation commented
Status: Needs work » Needs review
FileSize
cleanup-class-names-2650124-4.patch12.55 KB

Updated patch with -M. I have updated @file block, Please let me know if something missing.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ for Acquia commented
Status: Needs review » Needs work

Awesome, the patch with -M makes it a lot easier to review. Thanks :-)

The IS states not to rename the Trait tho, so let's revert that. Otherwise this looks good.

rashid_786’s picture

rashid_786 CreditAttribution: rashid_786 at SDG Corporation commented

"The IS states not to rename the Trait tho," You mean not to rename SearchApiFilterTrait.php right?

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ for Acquia commented

I think that's what @drunken monkey means, yeah.

rashid_786’s picture

rashid_786 CreditAttribution: rashid_786 at SDG Corporation commented
Status: Needs work » Needs review
FileSize
cleanup-class-names-2650124-8.patch10.28 KB

Updated patch with required changes.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ for Acquia commented
Status: Needs review » Reviewed & tested by the community

Looks great!

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
CreditAttribution: drunken monkey for Acquia commented
Status: Reviewed & tested by the community » Fixed

I agree, looks good. Thanks!
Committed.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
CreditAttribution: drunken monkey for Acquia commented
Component: Plugins » Views integration

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.