Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
CVS HEAD come with some not proper line break or empty line, which can cleanup with scripts/code-clean.sh.
Patch via CVS HEAD, directly apply scripts/code-clean.sh without any other change.
Comment | File | Size | Author |
---|---|---|---|
#30 | drupal-HEAD-1284611436.patch | 32.28 KB | hswong3i |
#27 | drupal-HEAD-1284428012.patch | 232.98 KB | hswong3i |
#25 | drupal-HEAD-1284428012.patch | 259.37 KB | hswong3i |
#22 | drupal-6.x-1284382718.patch | 25.78 KB | hswong3i |
#18 | drupal_code_clean-1226500171.patch | 8.71 KB | hswong3i |
Comments
Comment #1
hswong3i CreditAttribution: hswong3i commentedJust coding style cleanup with scripts/code-clean.sh. Should able to be RTBC directly?
Comment #2
chx CreditAttribution: chx commentedIn case you do not see what gets changed in the patch , click select all to highlight it. In this very nice patch we get rid of a number of trailing whitespaces. Nice job.
Comment #3
Dave ReidPatch applied cleanly and always +1 for core coding consistency!
Comment #4
catchNo longer applies cleanly.
Comment #5
Dave ReidRe-rolled. Did searches for the following:
Replaced tabs with spaces: '\t' => ' '
Removed trailing spaces: '[ \t]+$' => ''
Checked for un-even indents (i.e. three spaces instead of two): [^( )+ [^\* ]
Found a few array-indentation & trailing comma issues as well
I should really just try and get coder working on HEAD. :)
Comment #6
Dave ReidHoly crap. What we're fixing:
- Trailing spaces at end of lines
- Tabs and mis-indentations
- Space after control operators (like if, else, switch, etc)
- Space between ) and {
- STRING CONCATENATION
I started running all tests and I'm also checking through the patch line by line to make sure I didn't make any mistakes.
NOTICE: There were no kittens harmed during this patch process.
Comment #7
Dave ReidAll tests ran. Passed 100% (except for that damn upload test which passes for everyone else). Patch applies cleanly. No change in functionality - just coding standards. This should be ready, but webchick should have final approval.
Comment #8
Dave ReidRe-rolled with the rollback of drupal_set_title stuff.
Comment #9
webchickA couple of these look a bit odd and I don't think were caught by your regex fu. From bottom to top because that's how I read huge patches liek this. :P
If we're out-denting the function, we probably need to out-dent its PHPDoc as well, no?
Hm. That one got outdented one level more than the parent bracket did. Mistake?
While we're at it, there shouldn't be a space after $value.
same outdenting of PHPDoc problem here.
Comment #10
webchickOk. Didn't quite make it this time. We'll try for this at the end of UNSTABLE-3. Hopefully as separate, smaller patches that each do one thing and are easy to bang through and review. :)
Comment #11
Dave ReidAgreed. I'll be on top of my game next time! Same bat time, same bat issue queue!
Comment #12
hswong3i CreditAttribution: hswong3i commentedShould we just keep the work as simple as possible? I think cleanup with scripts/code-clean.sh is totally enough at this moment ?_?
Comment #13
hswong3i CreditAttribution: hswong3i commentedPatch reroll via CVS HEAD.
Comment #14
hswong3i CreditAttribution: hswong3i commentedPatch reroll via CVS HEAD.
P.S. Should we always apply scripts\code-clean.sh before commit?
Comment #15
cburschkaGood idea...
Also, applies and passes, so let's get this in before it breaks again.
(I hate patches that are so big they need to get re-rolled on every tiny little commit...)
Comment #16
cburschkacommon.test was fixed already. Here's a rerun.
Comment #17
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks! :)
Comment #18
hswong3i CreditAttribution: hswong3i commentedWell, patch reroll via CVS HEAD.
Comment #20
webchickSubmitting for re-testing after snafu earlier
Comment #22
hswong3i CreditAttribution: hswong3i commentedA tiny fix to drupal-6.x-dev CVS with scripts/code-clean.sh
Comment #23
Dave ReidWould need to be fixed in 7.x first.
Comment #25
hswong3i CreditAttribution: hswong3i commentedPatch re-roll via CVS HEAD. Run code-clean.sh directly, and additional fix with modules/system/system.tar.inc comment syntax.
Comment #26
chx CreditAttribution: chx commentedmodules/system/system.tar.inc the PEAR class is off limits. We are not touching it.
Comment #27
hswong3i CreditAttribution: hswong3i commentedRetouch the patch that remove all changes for modules/system/system.tar.inc, and nothing else from #25.
P.S. How about misc/ui/jquery.*.js? Should we also keep as untouch, or tidy with our coding syntax?
Comment #28
hswong3i CreditAttribution: hswong3i commentedComment #29
moshe weitzman CreditAttribution: moshe weitzman commentedplease don't fork jquery ui just for whitespace and other coding standards trivia. thats silly from a maintenance point of view.
Comment #30
hswong3i CreditAttribution: hswong3i commentedPatch retouch from #27, re-roll with latest CVS HEAD, remove all changes to misc/ui/jquery*.
Comment #32
hswong3i CreditAttribution: hswong3i commented#30: drupal-HEAD-1284611436.patch queued for re-testing.
Comment #33
lambic CreditAttribution: lambic commentedPatch doesn't apply
Comment #34
TravisCarden CreditAttribution: TravisCarden commentedSee #1518116: [meta] Make Core pass Coder Review.