Closed
Bug 697695
Opened 13 years ago
Closed 10 years ago
Coalesce some Places source files
Categories
(Toolkit :: Places, defect)
Toolkit
Places
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: mak, Unassigned)
Details
(Whiteboard: [mentor=mak][good next bug])
Attachments
(1 file)
73.98 KB,
patch
|
Details | Diff | Splinter Review |
There are more than 60 files in the componen folder, some of these are pretty much small and not really useful by themselves. I'd like to coalesce some sources. My suggestions: nsMaybeWeakPtr.cpp/h => Helpers.cpp/h nsPlacesMacros.h => Helpers.h PlaceInfo.cpp/h => Helpers.cpp/h VisitInfo.cpp/h => Helpers.cpp/h nsPlacesIndexes.h => Schema.h nsPlacesTables.h => Schema.h nsPlacesTriggers.h => Schema.h SQLFunctions.cpp/h => Schema.cpp/h this should remove about 10 files from the global count, without losing clarity, Schema is an even clearer concept, Helpers is already a shared code module.
Reporter | ||
Comment 1•13 years ago
|
||
and likely rename AsyncFaviconHelpers.cpp/h to Favicons.cpp/h to follow History.cpp/h style.
Reporter | ||
Comment 2•12 years ago
|
||
Not working on this, it's a good first bug though.
Assignee: mak77 → nobody
Status: ASSIGNED → NEW
Whiteboard: [good first bug][mentor=mak]
Comment 3•11 years ago
|
||
please review the patch thoroughly, C++ is not my first language!
Attachment #719061 -
Flags: review?(mak77)
Comment 4•11 years ago
|
||
Marco: I did not rename AsyncFaviconHelpers.cpp/h. I could still do that, if you'd like.
Reporter | ||
Comment 5•11 years ago
|
||
I'm delaying this a bit cause it would bitrot the deprecation patches, and those have an higher priority atm. Looking at the patch it's likely I'll take only some pieces of it, mostly to avoid overwriting all of the blame, originally I didn't think enough to consequences of so many changes on blame information. So I will probably request to split the patch into pieces once I decide which parts we'll take. At first glance I'd say the new plan may be: nsPlacesMacros.h => Helpers.h (in this order) nsPlacesTables.h => Schema.h nsPlacesIndexes.h => Schema.h nsPlacesTriggers.h => Schema.h AsyncFaviconHelpers.cpp/h => Favicons.cpp/h This should just lose less important blame. But I suggest waiting for bug 834457 to avoid bitrotting each other.
Reporter | ||
Comment 6•11 years ago
|
||
Comment on attachment 719061 [details] [diff] [review] files coalesced according to comment 0 clearing per previous comment.
Attachment #719061 -
Flags: review?(mak77)
Updated•10 years ago
|
Whiteboard: [good first bug][mentor=mak] → [mentor=mak]
Updated•10 years ago
|
Whiteboard: [mentor=mak] → [mentor=mak][good next bug]
Reporter | ||
Comment 7•10 years ago
|
||
I think I'm going to wontfix this to avoid losing blame info, sorry.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•