Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
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.