Closed Bug 1125588 Opened 10 years ago Closed 8 years ago

Code quality problems in various components

Categories

(Firefox Build System :: General, defect, P5)

defect

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: coolypf, Unassigned)

References

Details

Due to unified compilation being enabled by default, there are code quality problems in various components.
Blocks: 1066878
Blocks: 1120069
Blocks: 650161
Blocks: 1100335
Blocks: 949703
Component: DOM → Build Config
Yuan, what are the "code quality problems" specifically? Please include as much detail as you can. The list of blocking bugs in this bug seems like a random list of bugs that has nothing to do with unified builds.
Flags: needinfo?(coolypf)
Whiteboard: [INVALID?]
My interpretation is that Yuan is finding nonunified compilation errors, tracking them back to the bug that introduced them, attaching fixes to those bugs, and marking those bugs as blocking this one. (In other words, patching the problems in the original bugs and recording everything encountered in this metabug.) I don't know if that's the right way to go about it or not, but if it is, we should probably rename this bug to "[meta] non-unified compilation errors" or "[meta] namespacing errors" or something. I'm happy that Yuan is fixing them, though!
We probably need separate new bugs for the fixes, on the one-bug-per-problem basis.
Yes, we need separate bugs for the fixes. Thanks, Yuan!
(In reply to :Ehsan Akhgari [Away: 1/29-2/20] (not reading bugmail, needinfo? me!) from comment #1) > Yuan, what are the "code quality problems" specifically? Please include as > much detail as you can. The list of blocking bugs in this bug seems like a > random list of bugs that has nothing to do with unified builds. Here is the logic: unified build enabled by default .cpp/.c files merged together during compilation #include and using namespace in a.cpp take effect in b.cpp people forget adding #include and using namespace when modifying b.cpp since the compiler does not complain the correctness of source code depends on the somewhat unstable unified build system
Flags: needinfo?(coolypf)
(In reply to Yuan Pengfei from comment #5) > the correctness of source code depends on the somewhat unstable unified > build system Proof of unstableness: Bug 1043701 Comment 7 Bug 1043701 Comment 8 Bug 1112945
(In reply to Steve Fink [:sfink, :s:] from comment #2) > My interpretation is that Yuan is finding nonunified compilation errors, > tracking them back to the bug that introduced them, attaching fixes to those > bugs, and marking those bugs as blocking this one. (In other words, patching > the problems in the original bugs and recording everything encountered in > this metabug.) > > I don't know if that's the right way to go about it or not, but if it is, we > should probably rename this bug to "[meta] non-unified compilation errors" > or "[meta] namespacing errors" or something. > > I'm happy that Yuan is fixing them, though! It seems that unified builds are also affected...
(In reply to Boris Zbarsky [:bz] from comment #3) > We probably need separate new bugs for the fixes, on the one-bug-per-problem > basis. Yeah. I think the proper way is to file new bugs blocking the original ones as well as this one. However, it is a little complicated for me to do that... Bug 650161 -- blocked by Bug xxx \ Bug 1096089 -- blocked by Bug yyy -- blocking Bug 1125588 Bug 949703 -- blocked by Bug zzz /
(In reply to Yuan Pengfei from comment #8) > (In reply to Boris Zbarsky [:bz] from comment #3) > > We probably need separate new bugs for the fixes, on the one-bug-per-problem > > basis. > > Yeah. > I think the proper way is to file new bugs blocking the original ones as > well as this one. > However, it is a little complicated for me to do that... > > Bug 650161 -- blocked by Bug xxx \ > Bug 1096089 -- blocked by Bug yyy -- blocking Bug 1125588 > Bug 949703 -- blocked by Bug zzz / No, you just need to make bug xxx for example block both bug 650161 and bug 1125588.
Whiteboard: [INVALID?]
Priority: -- → P2
(In reply to Boris Zbarsky [:bz] from comment #3) > We probably need separate new bugs for the fixes, on the one-bug-per-problem > basis. Alternatively, how about we just give these fixes blanket r+, and land them without bugs? "No bug. Add missing header. r=unified" or if you prefer a name, feel free to use "No bug. Add missing header. r=sfink" as long as it's just a header addition or namespace qualification.
(In reply to Steve Fink [:sfink, :s:] from comment #10) > (In reply to Boris Zbarsky [:bz] from comment #3) > > We probably need separate new bugs for the fixes, on the one-bug-per-problem > > basis. > > Alternatively, how about we just give these fixes blanket r+, and land them > without bugs? > > "No bug. Add missing header. r=unified" > > or if you prefer a name, feel free to use > > "No bug. Add missing header. r=sfink" > > as long as it's just a header addition or namespace qualification. Might as well mention the bug that broke it: "Fixup for bug xxx. Add missing header. r=yyy"
Priority: P2 → P5
All the code quality problems in Firefox have been fixed! But seriously, this looks like a tracking bug and all dependencies are closed. So resolving.
Status: UNCONFIRMED → RESOLVED
Closed: 8 years ago
Resolution: --- → WORKSFORME
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.