Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.
I personally don't use a CHANGELOG.txt as a maintainer, but for those that do, I don't see any reason to force them to wrap commit messages at 80 characters.
The coding standards say "In general, all lines of code should not be longer than 80 chars."
README.txt is written by a person, and asking them to wrap text at 80 chars is reasonable. But not when pasting in something from a git log.
But purely for making this a useful tool, CHANGELOG.txt happens to be the first file alphabetically in many projects, meaning you have to scroll all the way past it just to see useful information.
Wait, wait, why do we check only PHP code and where was that decided?
Scanning .txt files was done for two reasons:
* check that the line length does not exceed 80 characters.
* check that the file ends in a single newline.
If you do not want to scan .txt files you can just simply not include them in your --extensions setting when invoking phpcs.
Also, I want to scan CSS files in the future to validate them against our CSS coding standards. Are you saying that we should not do that either (phpcs is designed to handle that)?
The whole reason for tools is to use the same tools the php community uses for checking against is coding standards.
The css and javascript community also have tools they use to check their css/js and those tools should be used for that. Sadly the .txt community doesn't really exist.
Essentially this is a small simple tool, does one thing very well. Anything outside that scope should be handled by different tools.
Does this make sense?
I would love for all of us to come to an agreement into what we cover and what we don't. The tool isn't really useful is it starts covering preferences. :-/
phpcs can handle CSS and JS files very well, see the existing sniffs of other standards targeted at that. We should leverage them.
.txt is an edge case, but .txt is mentioned nowhere in ruleset.xml. It just depends on how you invoke phpcs, so there is no harm by keeping the .txt sniffs.
Edit: oh, .txt was mentioned in ruleset.xml, but only as exclusion pattern that triggers if you actually scan .txt files. So really no harm done here.
I know css and js work very well with phpcs but that doesn't mean it should be the tool to use. As well as it works it's not the greatest. For javascript most people should be using either jslint or jshint. For css I'm not extremely familiar with those tools but most people would probably use css lint.
The reason for using those tools over this one is the same reason this project was started. To not have a completely homegrown solution to a very common problem and instead use the tools the community is using for the specific problem be it js/css/ or php.
Txt is indeed an edge case but the same principles applies.
Interesting, I have somwhat different reasons why I joined this project: I needed a parser because regexes just don't get the job done (see coder). In addition PHPCS has many rules that we can use 1:1.
The problem that I see now is the following: we will have to integrate 3 different libraries (phpcs, jslint, css lint), then re-create common rules in all libraries (2 space indentation, no white spaces at the end, file endings in single new line, etc.). And of course we have to learn 3 different libraries to write new rules.
But this does not stop here, I want to make use of automated code reviews for project applications, so I have to:
* Maintain pareview.sh that invokes all code review tools
* Maintain drupalcs that uses phpcs for PHP code review
* Maintain drupaljslint that uses jslint for JS code review
* Maintain drupalcsslint that uses csslint for CSS code review
* Maintain drupaltxt that uses phpcs for TXT file review
* Maintain drupalinfo that uses phpcs for INFO file review
Phew, I will not get many actual project applications done at the end of the day ...
2 questions:
Where do I put the sniff for TXT files now? Should I really start a new project for that? Should I move the rules back to ugly bash hacks in pareview.sh?
What is the problem if the TXT sniff just stays in drupalcs and we just invoke phpcs differently? You use
My point of view is similar to klausis.
I'd say we should build those sniffs, at leas as long as there isn't a overtaking project dedicated for those sniffs.
I try to be very pragmatic in such things: If there's no better solution now and if it's a low hanging fruit. Do it, but drop it as soon as condition one applies.
Especially if we're talking about stuff that mainly affects dev's we should be able to expect this kind of flexibility.
For me this comes down to "Peaceful co-existence.", which means we need to:
Agree on a default setup. But do we really need to change PHP_CodeSniffer's defaults? If you don't pass txt as extension / or explicitly sniff a txt file it shouldn't scan it. Or am I wrong?
I can't really argue with peaceful co-existence can I :-p
Yea, most of it might just have to be on documentation.
I'm going to revert my last commit. We'll need to start documenting the sniff better.
So now I'm perfectly fine with just changing my alias to include my selected file extensions. We'll just need to document it for every else knows how this all works.
Comments
Comment #1
klausiWhy? Can you give an example of a line that you would like to exceed 80 characters?
Comment #2
tim.plunkettI'm not going to waste my time explaining the obvious.
Comment #3
klausiWell, maybe someone else can explain it to me.
Comment #4
tim.plunkettSee http://drupalcode.org/project/date.git/blob/refs/heads/7.x-2.x:/CHANGELO...
I personally don't use a CHANGELOG.txt as a maintainer, but for those that do, I don't see any reason to force them to wrap commit messages at 80 characters.
The coding standards say "In general, all lines of code should not be longer than 80 chars."
README.txt is written by a person, and asking them to wrap text at 80 chars is reasonable. But not when pasting in something from a git log.
But purely for making this a useful tool, CHANGELOG.txt happens to be the first file alphabetically in many projects, meaning you have to scroll all the way past it just to see useful information.
Comment #5
klausiThe point of having a CHANGELOG.txt is to have a pretty formatted summary, IMO. Otherwise I could just look at the git log, no?
And http://drupal.org/node/447604 says "For instance, it is recommended and generally accepted that the file should wrap at 80 columns".
Comment #6
ericduran commentedI have to agree, I don't understand why we added .txt to scan.
The point of drupalcs is strictly to only scan the php code and that's it. Everything else can and should be handled separately.
Comment #7
ericduran commentedWe have to remember the Goal of this project is to help developers follow the coding standards when it comes to php code.
Strictly php code. Anything else can and should be left to other projects . :)
This is fixed. I removed the txt sniff.
http://drupalcode.org/project/drupalcs.git/commit/d7377a0
Comment #8
klausiWait, wait, why do we check only PHP code and where was that decided?
Scanning .txt files was done for two reasons:
* check that the line length does not exceed 80 characters.
* check that the file ends in a single newline.
If you do not want to scan .txt files you can just simply not include them in your --extensions setting when invoking phpcs.
Comment #9
ericduran commentedThis has always been the goal of this project, the .txt files is beyond the scope of the code sniffer especially since it's not actual code.
Comment #10
klausiAlso, I want to scan CSS files in the future to validate them against our CSS coding standards. Are you saying that we should not do that either (phpcs is designed to handle that)?
Comment #11
ericduran commentedphpcs was meant for php code.
The whole reason for tools is to use the same tools the php community uses for checking against is coding standards.
The css and javascript community also have tools they use to check their css/js and those tools should be used for that. Sadly the .txt community doesn't really exist.
Essentially this is a small simple tool, does one thing very well. Anything outside that scope should be handled by different tools.
Does this make sense?
I would love for all of us to come to an agreement into what we cover and what we don't. The tool isn't really useful is it starts covering preferences. :-/
Comment #12
ericduran commentedOne things we can do is have multiple standards in the download and that other standard can include non standard sniff that most people would not use.
But we really shouldn't do these kind of stuff on the regular ruleset.xml.
Comment #13
klausiphpcs can handle CSS and JS files very well, see the existing sniffs of other standards targeted at that. We should leverage them.
.txt is an edge case, but .txt is mentioned nowhere in ruleset.xml. It just depends on how you invoke phpcs, so there is no harm by keeping the .txt sniffs.
Edit: oh, .txt was mentioned in ruleset.xml, but only as exclusion pattern that triggers if you actually scan .txt files. So really no harm done here.
Comment #14
ericduran commentedI know css and js work very well with phpcs but that doesn't mean it should be the tool to use. As well as it works it's not the greatest. For javascript most people should be using either jslint or jshint. For css I'm not extremely familiar with those tools but most people would probably use css lint.
The reason for using those tools over this one is the same reason this project was started. To not have a completely homegrown solution to a very common problem and instead use the tools the community is using for the specific problem be it js/css/ or php.
Txt is indeed an edge case but the same principles applies.
Comment #15
klausiInteresting, I have somwhat different reasons why I joined this project: I needed a parser because regexes just don't get the job done (see coder). In addition PHPCS has many rules that we can use 1:1.
The problem that I see now is the following: we will have to integrate 3 different libraries (phpcs, jslint, css lint), then re-create common rules in all libraries (2 space indentation, no white spaces at the end, file endings in single new line, etc.). And of course we have to learn 3 different libraries to write new rules.
But this does not stop here, I want to make use of automated code reviews for project applications, so I have to:
* Maintain pareview.sh that invokes all code review tools
* Maintain drupalcs that uses phpcs for PHP code review
* Maintain drupaljslint that uses jslint for JS code review
* Maintain drupalcsslint that uses csslint for CSS code review
* Maintain drupaltxt that uses phpcs for TXT file review
* Maintain drupalinfo that uses phpcs for INFO file review
Phew, I will not get many actual project applications done at the end of the day ...
2 questions:
Where do I put the sniff for TXT files now? Should I really start a new project for that? Should I move the rules back to ugly bash hacks in pareview.sh?
What is the problem if the TXT sniff just stays in drupalcs and we just invoke phpcs differently? You use
And I use
Peaceful co-existence.
Comment #16
das-peter commentedMy point of view is similar to klausis.
I'd say we should build those sniffs, at leas as long as there isn't a overtaking project dedicated for those sniffs.
I try to be very pragmatic in such things: If there's no better solution now and if it's a low hanging fruit. Do it, but drop it as soon as condition one applies.
Especially if we're talking about stuff that mainly affects dev's we should be able to expect this kind of flexibility.
For me this comes down to "Peaceful co-existence.", which means we need to:
Comment #17
ericduran commentedI can't really argue with peaceful co-existence can I :-p
Yea, most of it might just have to be on documentation.
I'm going to revert my last commit. We'll need to start documenting the sniff better.
So now I'm perfectly fine with just changing my alias to include my selected file extensions. We'll just need to document it for every else knows how this all works.
Comment #18
ericduran commentedReverted -- http://drupalcode.org/project/drupalcs.git/commit/dd251e7
Also changing title to reflect work needed.
Comment #19
klausiI added an additional usage example to the project page.
Comment #20
klausiUsage documentation regarding file extensions lives at http://drupal.org/node/1587138 . I think we can consider this as fixed, reopen if you disagree.