Closed
Bug 673499
Opened 13 years ago
Closed 12 years ago
Remove Whitespace
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: evilpie, Assigned: evilpie)
Details
Attachments
(1 file, 2 obsolete files)
177.71 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•13 years ago
|
||
Assignee | ||
Comment 2•13 years ago
|
||
Attachment #547762 -
Attachment is obsolete: true
Attachment #547792 -
Flags: review?(Ms2ger)
Comment 3•13 years ago
|
||
Comment on attachment 547792 [details] [diff] [review] remove trailing whitespace Leave ctypes/libffi.patch and ctypes/libffi/* alone. If you're going to touch /js/src/configure.in, check if you need to update /configure.in as well. Maybe leave xpconnect/ alone as well, and yarr/ if we update it from upstream. r=me with those changes, but keep in mind I'm technically not qualified to review stuff.
Attachment #547792 -
Flags: review?(Ms2ger) → review+
Comment 4•13 years ago
|
||
Comment on attachment 547792 [details] [diff] [review] remove trailing whitespace This patch is too broad. I'd review a patch restricted to C/C++ files in js/src and the subdirectories js/src/{vm,methodjit,tracejit,shell}. I'd also be willing to review a separate patch restricted to js/src/{jit-test,tests,jsapi-tests}, but it's maybe not such a bad thing to have a few tests that contain trailing whitespace; some of those may even be intentional. Much of the rest is code other teams are responsible for, sometimes copied from upstream sources. We should not make unnecessary changes there.
Attachment #547792 -
Flags: review+ → review-
Comment 5•13 years ago
|
||
If it helps at all, the js/src/ build system whitespace cleanup will be dealt with by Matheus' patch in bug 671465.
Assignee: general → evilpies
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•12 years ago
|
||
Attachment #547792 -
Attachment is obsolete: true
Attachment #624398 -
Flags: review?(jorendorff)
Comment 7•12 years ago
|
||
Comment on attachment 624398 [details] [diff] [review] smaller patch Review of attachment 624398 [details] [diff] [review]: ----------------------------------------------------------------- r=me with one comment. ::: js/src/ETWProvider.man @@ +23,5 @@ > <event symbol="EvtExecuteDone" value="1002" version="1" > channel="MozillaChannel" level="win:Informational" > template="CodeLocationTemplate" task="Execution" opcode="ExecuteStop" > keywords="SampleKeyword" message="$(string.MozillaSpiderMonkey.ExecuteDone.message)"/> > + I have no idea what this file is. Surely this is autogenerated by some Microsoft tool that puts in the useless spaces, and the spaces will only come back the next time this is updated. In any case you don't want your name on this file's changelog. Please revert this one.
Attachment #624398 -
Flags: review?(jorendorff) → review+
Assignee | ||
Comment 8•12 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/c80dde3faa3d And reverting the change to js/src/ETWProvider.man, which I accidentally did not remove after some rebasing action. http://hg.mozilla.org/integration/mozilla-inbound/rev/71889cff519c
Assignee | ||
Updated•12 years ago
|
Target Milestone: --- → mozilla15
Comment 9•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c80dde3faa3d
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Comment 10•12 years ago
|
||
And follow-up: https://hg.mozilla.org/mozilla-central/rev/71889cff519c
You need to log in
before you can comment on or make changes to this bug.
Description
•