Code review comment for lp:~allenap/maas/repackage

Reading a bit further. Getting a bit lost with all the repetition! Is there really no way to share common definitions of __version__, envvar, check_settings, and so on? It's a lot of stuff to have both duplicated and untested, even if it is "meta" code.

.

I can see how it could be useful in the future for envvar to yield the environment variable's original value. But it's also a potential source of misunderstanding: "with environment variable set to X, its value is Y." I would just leave that feature out until we actually need it.

.

Much of the duplicated code needs documentation, and I can't help feeling that the duplication is the reason for not documenting it — which would be a poor state of affairs. I'm thinking of check_settings in particular: which settings does this check, for whose benefit, to serve what need?

.

Why doesn't install_deps follow our naming conventions?

.

I think the linter needs an update to check the pkg directory. The setup() calls are on insanely long lines.

.

Does pkg/common/checkarchive.py really compare contents of a tarball to a directory? Or only the file and directory names?

.

In pkg/maas-cluster/provisioningserver/testing/__init__.py you define "here" and "root" as global variables. Better upper-case them, as we have mostly been doing. Moreover "root" needs a more descriptive name and a bit of documentation. Root of what? And in a branch, in a package installation, or both?