Hi Danek & Brock, Thanks for the reviews,
On Fri, 2011-09-02 at 16:57 -0700, Danek Duvall wrote: > Tim Foster wrote: > > > https://cr.opensolaris.org/action/browse/pkg/timf/pkglint_whinge_less/pkglint_whinge_less-webrev/ > > pkglint.py: > > - line 278: I think I'd recommended to Shawn in a previous review not to > name build_release, and just treat it as an optional positional > parameter, so I'll do the same here. But why not just put "5.11" in > the first attempt at object creation? The parameter is only used if > the version isn't in the fmri string. I didn't know that - I'll make the change to use a positional parameter, and always use 5.11. > - line 283: str() call isn't necessary, given that it's for a %s. Thanks, fixed. On Fri, 2011-09-02 at 16:48 -0700, Brock Pytlik wrote: > I have general concerns about fixing bug 18673, at least as generally as > described. I don't think that all packages linted should need to contain > the same set of declared variants, but I do think that if a package has > an action which uses a particular variant (debug or not) it should > declare that variant in a set action. Ok, so that's the problem people were describing in the bug. The original spec for pkglint said that variant.debug.* was special, but I'm unsure of the original intent. The upshot of that decision (and my not getting the spec right originally :), was that that variant wasn't declared in the ON manifests that used it, and pkglint got upset about that. > What's missing for me (in bug > 18673 and the associated mails) is an explanation of why we want to skip > debug variants. We have one case where it actually caught a bug that > would've gone unnoticed otherwise (bug 18884). A fair question - I'd like to know the answer too. In order for the other changes in this wad to make it back, particularly the change for 18808 which we'd like to improve the build (and is blocking some work David was looking to do) I'll drop the variant.debug.* fix for now, and just putback the others. Thanks again for the reviews. cheers, tim _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
