On 08/06/13 09:49, Erik Trauschke wrote:
On 08/ 5/13 01:39 PM, Bart Smaalders wrote:
OK; I'm happy enough with this wad at this point; we prob. need to
get some more user feedback.
Just FYI, the latest webrev was here:
I had put that in a new thread since I consolidated all the info from
the code review and design discussion threads.
I would like to suggest the following new bug summary for 16992741:
want publication tool to minimize package version changes
Can you file a separate bug for these just to highlight the fix?
line 221: A much more efficient method of getting these is:
lines 403-436: Do these really need to be nested functions or can
they be standalone and take more parameters? If they weren't
nested, I'd definitely combine them for a big performance win.
Speaking of which, how long does a pkgsurf run take now?
line 412: ??? Should easily be able to compare these as a set of
strings. require-any is the common use case here. If this is really
just "too hard right now" and waiting to fix this later, that's ok.
line 458: Again, seems like this could be a standalone function
instead of nested. There's a nasty perf penalty for nested
functions. In this particular case, I'm not even sure why it's
a function when it could just as easily be in-lined.
line 655: s/return None/return/ (They're equivalent in Python)
lines 706-707: The unfortunate thing about this is that I think this
will unintentionally make incorporate dependencies of reversioned
packages more specific than they were originally. A way around that
would be to see if the package is still allowed by the old
incorporate assume it doesn't need updating. If it does, bonus
points for keeping the same level of specificity. Don't know
how to do that yet though. Will talk offline to you about this
lines 741-777: Lots of this looks like adjust_dep_action; no way
to make common?
general: want to see tests for valid repo specified,
empty target repo specified,
empty source and empty target repo specified,
general comment: wonder if we should have a separate exit code for "no
changes made" to make that easy to detect in a script, just as pkg(1)
has exit 4 for no updates available
[pkg-discuss] Re: small code review: 16992741 New publication tool, which reverts pkgs in repo if content hasn't changed