search

civicrm / coder

Code sniffer configuration for CiviCRM. (Relaxed variant of Drupal CS)
5 stars 11 forks source link

Propose re-instate forcing a file comment to exist #4

Closed eileenmcnaughton closed 5 years ago

agh1 commented 7 years ago

@eileenmcnaughton To clarify, did you mean "file comment"? I think your change makes function comments required. (I also think it's a good change to require function comments, but I just wanted to be sure that's what you mean.)

eileenmcnaughton commented 7 years ago

yeah - that was the intent. We haven't confirmed for sure how many issues it will cause yet. Through phpstorm I found & fixed a bunch & so did TIm but there are probably still some that have been missed

totten commented 5 years ago

FWIW, at time of writing, there would be 2000 issues if we activate Drupal.Commenting.FunctionComment.Missing. That's a lot of comments to write. Maybe if there was some way to roll that out progressively (including some folders; excluding others)?

Activating Drupal.Commenting.FileComment.Missing would require a different patch, but it would only raise 20 issues.

eileenmcnaughton commented 5 years ago

@totten I though in the past you indicated it wasn't possible to role out incrementally? I would love to do that - would be happy to tackle a few folders....

totten commented 5 years ago

Right - I don't see a simple way to roll out incrementally. But these tools change over time, and sometimes someone comes up with a clever technique.

I personally don't have anything clever right now -- there's just the Unclever Way: write a better script-thing which wraps around phpcs and applies different policies in different cases. Given that we're adding to a scope which already includes multiple repos/projects/file-structures as well as local+CI usage, we're probably looking at... at least a day's work (of various sorts).

eileenmcnaughton commented 5 years ago

@totten from memory the coder stuff was originally supposed to allow different standards for some folders - but it didn't work?

totten commented 5 years ago

I don't think phpcs / coder has ever had a feature for enforcing different standards in different folders -- but you can be selective about how you call it. In civilint, there's a set of adhoc rules for skipping certain messy files (like auto-generated DAOs and examples). One could imagine using a wrapper-script or a future/revised phpcs which enforces different rules for different files based on either:

(Aside: Along the lines of "folders" and "QA", I had done some sketches/experiments around triggering different test-suites based on which folders changed but never put enough time in to finish it. Didn't consider phpcs as a test suite at the time, but that wouldn't be a huge leap.)