[sugar] [PATCH] Refactor bundlebuilder
Tomeu Vizoso
tomeu
Tue May 27 10:25:14 EDT 2008
Hi,
didn't found many interesting things, so I had to invent some new nitpicks:
info_path = os.path.join(config.source_dir, 'activity', 'activity.info')
f = open(info_path,'r')
info = f.read()
f.close()
exp = re.compile('activity_version\s?=\s?([0-9]*)')
match = re.search(exp, info)
version = int(match.group(1)) + 1
info = re.sub(exp, 'activity_version = %d' % version, info)
f = open(info_path, 'w')
f.write(info)
f.close()
Why not using the ConfigParser to read and modify that file instead?
retcode = subprocess.call(['git', 'commit', '-a', '-m % s' % changelog])
Surprised that '% s' % changelog works. May be more expected to use
'%s' instead?
retcode = subprocess.call(['git', 'commit', '-a', '-m % s' % changelog])
if retcode:
print 'ERROR - cannot commit to git'
This will commit other commits that may be pending in the local tree.
Perhaps we should check at the start of this function with git-status
that no changes are pending? Somebody suggested that recently in some
other thread.
retcode = subprocess.call(['git', 'push'])
if retcode:
print 'ERROR - cannot push to git'
Similarly, we may want to check before that the local tree is in sync
with the remote, so we don't push other commits.
zf = zipfile.ZipFile(packager.package_path)
for name in zf.namelist():
full_path = os.path.join(path, name)
if not os.path.exists(os.path.dirname(full_path)):
os.makedirs(os.path.dirname(full_path))
outfile = open(full_path, 'wb')
outfile.write(zf.read(name))
outfile.flush()
outfile.close()
Why not just use unzip?
try:
os.symlink(config.source_dir, bundle_path)
except OSError:
if os.path.islink(bundle_path):
print 'ERROR - The bundle has been already setup for development.'
else:
print 'ERROR - A bundle with the same name is already installed.'
Should we check that the error is in fact "[Errno 17] File exists" and
not something else (no permission, non-existent dir, etc)?
ignore_dirs=[ 'locale', 'dist', '.git' ],
What's this thing about spaces enclosing the elements of a list? ;)
def package(self):
tar = tarfile.open(self.package_path, "w")
Extra spaces.
# Copyright (C) 2006-2007 Red Hat, Inc.
We are in 2008!
Regards,
Tomeu
More information about the Sugar-devel
mailing list