Mark J. Nelson writes:
> http://anthrax.central/net/mrliberal/export/ws/mjnelson/onnv-transition-dev/webrev.incremental.codereview/
Looks good.
> Because they were comments, and not generally user visible, I elected to
> honour the author's preferred spelling. :)
;-} Fine by me.
> > For the .java files, do the "@version" things matter in any way for
> > javadoc? If they weren't important, then just stripping them seems
> > good. Otherwise, we might need some other fix. (I'm asking because
> > [a] I'm no java expert and [b] CR 4758439 is still in 'Accepted' state
> > and doesn't have an Evaluation.)
>
> They "matter" to javadoc, in that they cause false positives on a wsdiff.
> I do not believe that they matter for anything useful.
The 'useful' part is what I was worried about. If they're not, then
that's fine.
> In reviewing this change initially, I checked for the absence, comment,
> or definition of PSTAMP in ON packages, and I found a sufficient number of
> each case that I didn't feel strongly, especially since "absent" and
> "commented" are functionally equivalent.
They're equivalent for packaging, but perhaps not for WOS scripts that
manipulate these things.
I'd imagine it's fine ... though it's hard to tell why someone went to
the trouble of adding piles of 'em in as commented out on other
packages. What was the point?
> > tools/env/opensolaris.sh:82: the "//hg" here doesn't look right to me.
> > All of my working paths to hg.opensolaris.org use a relative reference
> > to "hg/onnv/onnv-gate" and not an absolute path. Is this right?
>
> Because hg.os.o does a chroot, absolute and relative paths are
> functionally equivalent. Rich and I were surprised to find each other
> using different forms of the same path, but I tested both, and each works.
>
> I have a slight preference for the absolute, because I wonder if the
> chroot will someday go away, and if so, I think that means the paths won't
> need to change, as long as we keep the repos in the obvious location.
OK, that works for me.
> > I don't see how the change in tools/onbld/Scm/Backup.py is related to
> > bug ID 303.
>
> It's this single line (from the new file):
>
> 363 self.files = ('hgrc', 'localtags', 'patches', 'cdm')
>
> that adds the new .NOT files in the .hg/cdm/ directory to the list of
> files to be included in a backup.
Ah, ok. That's what I was missing.
--
James Carlson, Solaris Networking <james.d.carlson at sun.com>
Sun Microsystems / 35 Network Drive 71.232W Vox +1 781 442 2084
MS UBUR02-212 / Burlington MA 01803-2757 42.496N Fax +1 781 442 1677
