On 07/13/10 11:53 AM, [email protected] wrote:
Folks,
For some time now, the publication tools have needed an update to use
the transport system that we built for the client about a year ago.
This work allows the publication tools to use that same code,
eliminating a bunch of bloat, duplication, and bit-rot. These changes
are also needed so that Shawn can finish his on-disk-format work.
The webrev is here:
http://cr.opensolaris.org/~johansen/webrev-xpub-1/
======================================================================
src/modules/misc.py:
======================================================================
lines 30, 37, 52, 47, 41: httplib, urlhelpers, global_settings,
urllib2, socket are all unused now
======================================================================
src/modules/publish/transaction.py:
======================================================================
line 31: os import not used
======================================================================
src/publish.py:
======================================================================
line 54: this is already imported on line 50?
lines 289, 389: not your fault, but can you kill the semicolons?
line 499: not your fault, but can you fix indentation? (off by 1)
======================================================================
src/pull.py:
======================================================================
lines 48, 49: ptf and portable imports no longer used
======================================================================
src/modules/client/transport/repo.py:
======================================================================
line 598: missing spaces after '{' and before '}'. Also, it seems
like you could trivially add support for add to catalog here, by
(adding) and checking for X-IPkg-Add-To-Catalog.
line 600: Why not simply headers.update(header) ?
line 636: extra newline
lines 719-724: Need docstring bits for 'frepo' ?
line 1154: s/content to/an action and its payload (if applicable) to
an existing transaction in/ ?
line 1165: s/'abandon' publication operation, that/abandon operation/
line 1197: s/Returns a transaction-ID/Returns the identifier string
for the transaction/
======================================================================
src/modules/client/transport/transport.py:
======================================================================
lines 85, 123: insert newline here (two lines between classes)
line 125: missing docstring
line 127: s/misc.ImmutableDict()/misc.EmptyDict/
lines 142-143: This can be:
return (p for p in self.__publishers.values())
lines 146-148: This can be:
return self.__policy_map.get(policy_name, False)
lines 1779-1783: simplified to the following?
repolist = repo.origins[:]
if not origin_only:
repolist.extend(repo.mirrors)
line 1961: extra newline
line 2051: so why cycle through origins for publication operations?
it seems like we should never have more than one origin for
publication. Same for other publication opeartions on 2096, etc.
line 2474: list of files in filelist?
======================================================================
src/tests/cli/t_publish_api.py:
======================================================================
line 88: extra newline?
======================================================================
src/util/distro-import/importer.py:
======================================================================
line 23: Should really be 2009, 2010 I believe.
lines 43, 54: repository and urlparse imports no longer used
line 560: strictly speaking, 'global' isn't required if you're only
reading global variables; just assigning (see 'pydoc global').
This is done in other places too...
line 1406: replace 'lib/pkg.depotd' with 'bin/pkgrepo'; also update
docstring on line 1404
line 1407: replace with:
argstr = "%s -s %s refresh" % (cmdname, path_to_repo)
line 1468: this comment should probably move up to above 1456 or be
changed slightly
not your fault, but can you fix these?
line 1428: indentation off by one
======================================================================
src/util/publish/merge.py:
======================================================================
lines 182-231: The code in these seems common to our publication
utilities. Any way to centralise these somewhere? (misc? a new
module? perhaps pkg.util?) I realise the globals make this
difficult.
not your fault, but can you fix these?
line 41: ptf import no used
line 89: missing spaces around '='
lines 237, 251: indentation off by one
line 403: missing space after ','
Cheers,
-Shawn
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
