search

totten / civix

CiviCRM Extension Builder
http://civicrm.org/
Other
56 stars 56 forks source link

Standards fixes in civix #261

Closed eileenmcnaughton closed 2 years ago

eileenmcnaughton commented 2 years ago

I recently submitted a civix-generated extension & found man boilerplate lines didn't pass style - this fixes.

It also removes a 'not-a-word' 'myinstall' & switches to 'my_install' - we shouldn't make people have to add made-up words to their IDE spelling checkers...

https://github.com/civicrm/civicrm-core/pull/24585

totten commented 2 years ago

For the upgrade.php.php template, I could be misinterpreting - but it looks there's a bikesheddy thing around how to format a commented-out function, eg

/**
 * (1) Line-comments for entire function.
 */
// function helloWorld() {
//   echo "Hello world\n";
// }
/**
 * (2) Block comment for entire function
 *
function helloWorld() {
  echo "Hello world\n";
} // */
/**
 * (3) Line-comments for function body
 */
function helloWorld() {
  // echo "Hello world\n";
}

Most of the examples in this file are (1)'s. There were a couple (2)'s, which the PR changes to (3)'s.

There are small/edgy trade-offs... but overall, it seems preferable (easier to read) if the file is internally-consistent. I pushed switch the outliers to (1). Or is there a reason they need to be different?

eileenmcnaughton commented 2 years ago

Nah - they dont' need to be different - what you did is fine

eileenmcnaughton commented 2 years ago

Actually the need to NOT be 2 - for CI - but between 1 & 3 - both should be ok

totten commented 2 years ago

OK, cool.

Ran phpunit and tests/make-example.sh. The latter showed that it needed some new use statements. Pushed up.

Merging.