[illumos-Discuss] Little question about compilers and a future...

[Garrett D'Amore]

Just because something was known to run on Solaris before does not
automatically mean it gets an expedited review.  It *helps* the reviewer
do a simpler review, I suppose, but code review requirements still
exist. 

Projects that find this bar too high to cross should reconsider their
delivery into illumos/ON proper.  Perhaps separate delivery is better.

I appreciate your thoughts on how we should review projects, but they
don't match reality or current policy.

As far as cpp, 500 lines of external code is much easier to review than
the massive amount of code in star.

Finally, your priorities may be different than mine.  That is fine.  My
priorities are based on closing key functional gaps.  If you think my
priorities are incorrect, then you may appeal to developer-council by
sending a message to developer-council at lists.illumos.org.

Ultimately, if you can find another advocate (current advocates are gwr
and richlowe) to review your code and integrate, you don't need my
approval.  But in reality, I'm likely to be the most accommodating to
you in this regard.

	- Garrett


On Sat, 2010-09-18 at 16:20 +0200, Joerg Schilling wrote:
> "Garrett D'Amore" <garrett at damore.org> wrote:
> 
> > star is way down on the priority list.  I would hope you could
> 
> Star is at the same priority for me as cpp.
> 
> > understand why.  (The fact that its a giant webrev, imports a whole new
> > library interface, and need exceptional handling in our standard cstyle
> > rules, does not help here.  A low priority easy and low risk delta is
> > much easier to integrate right now than a low priority webrev with many
> > thousands of lines of change and requiring special handling.)
> 
> For the star integration, there are aprox. 50 lines to review. The rest is
> well tested code, developed on SunOS since more than 25 years. See below.
> 
> If there was a way to create a useful review for larger integrated projects
> from external, then there would be in theory a need to review 500 lines of 
> changes for cpp.
> 
> > If the warnings generated are due to whitespace conflicts, we should try
> > to fix those ... either by making them no longer conflicting in the
> > source, or by fixing your cpp.
> 
> Removing the warnings in cpp is not a trivial task. To ensure that there never 
> is a warning from different white space in a macro definition, the easiest 
> solution is to remove all whitespace while processing the definition. This 
> however introduces the risk of breaking code that depends on white space.
> This is something that would need a lot of testing. Only reducing the 
> probability of getting a a message:
> 
> xxx.h: 3: BLA redefined
> 
> is easy, but this ias no effects on a onnv compiolation.
> 
> Is someone able and willing to create a set of tests for the possoble problem 
> with removing spaces from macro definitions?
> 
> 
> I believe that after I fixed the fact based on the 1978 code that causes:
> 
> #if ..... /* XXX */
> 
> to cause a "syntax error" message if cpp is called with -C, the code could be 
> called formally correct.
> 
> 
> 
> > Of course, you've not posted a webrev, so your work is therefore private
> > and not useful to the rest of us.  At least not as far as integration in
> > illumos is concerned.
> 
> See above: the output from "webrev" is not useful for integrating external 
> projects. For an integration into illumos, let us do one step after the other 
> and first finish the star integration.
> 
> 
> 
> I encourage you to read my review cliassification......I send two weeks ago
> 
> -----
> Let me list some possible classes: 
>  
> problem not understood          We need to integrate stuff for a problem that 
>                                 is not fully understood 
>  
> 
> problem understood              We need to integrate stuff for a problem that 
>                                 is fully understood because the cause for the 
>                                 problem is well known and because the way to  
>                                 fix is well known 
>  
> big chunk of new software       This is a big chunk of new software that was 
>                                 not in Illumos before and that did not run 
>                                 on Solaris before 
>  
> big chunk of native software    This is a big chunk of software that was not 
>                                 in Illumos before, but that did run on Solaris 
>                                 already 
>  
> delta to existing software      This is a delta to software that did work  
>                                 previously in Illumos without bugs or with 
>                                 smaller bugs 
>  
> delta to integrating software   This is a delta to software that was previously 
>                                 integrated in Illumos but that did not work 
>                                 sufficiently 
> 
> Integrating star is "problem understood" + "big chunk of native software" 
> This is also a simple case, as the software is known to work on Solaris and  
> we only need to check the new Makefiles. 
> ---------
> 
> Jörg
>