[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#688437: RFS: loook/0.6.8-1 ( ITP )

(I don't intend to sponsor this package.)

* Mechtilde Stehmann <ooo@mechtilde.de>, 2012-09-22, 18:43:

Lintian reports:

I: loook source: quilt-patch-missing-description loook-python3.patch
W: loook source: debian-rules-missing-recommended-target build-arch
W: loook source: debian-rules-missing-recommended-target build-indep
I: loook source: debian-watch-file-is-missing
P: loook: no-upstream-changelog
W: loook: unknown-section office

README.Debian would be good place to mention how the Debian package diverges from upstream. But Python 3.X _is_ supported by upstream, so I don't feel it's useful to mention it in README.Debian.

Have you actually read your README.source?

What is "<688397 is the bug number of your ITP>" supposed to mean? :)

I'd use "debhelper (>= 8)" instead of "debhelper (>= 8.0.0)".

Current standards version is 3.9.4.

If you don't use VCS, remove the commented-out Vcs-* fields.

It's conventional to put a space after each comma in Depends.

Please ask debian-l10n-english@lists.debian.org for review of the package description and the manual page.

Please add get-orig-source target to your debian/rules. Why was COPYING not included in the .orig.tar.gz?

What are build-stamp and configure-stamp targets for? They don't seem to do anything useful...

What is "-rm -Rf tmp" for?

The install target could be much simplified if you didn't compress the manpages manually, but let dh_compress do it for you.
The package FTBFS when built twice in a row:

gzip -r9 /tmp/loook-0.6.8/debian/loook.man && \
mkdir -p debian/loook/usr/share/man/man1/
gzip: /tmp/loook-0.6.8/debian/loook.man: No such file or directory
make: *** [install] Error 1

I wouldn't use $(CURDIR) when it's not necessary; it makes build logs noisy for no good reason.

Now looking at upstream source:

if os.getenv('USERPROFILE'):
        config_path = os.getenv('USERPROFILE')
elif os.getenv('HOME'):
        config_path = os.getenv('HOME')

I recommend using: os.path.expanduser('~')

elif os.name == 'dos':
        config_path = "c:/"

This is dead code. DOS hasn't been supported since Python 2.3.

Application.saveConfig() saves the configuration always in UTF-8, but Application.__init__() would use locale encoding. Also, I think saveConfig() will fail badly if any of the settings contains special characters like # or %.

if os.name == 'nt':
                print("Warning: File could not be opened. - %s" % filename)
        prg = self.ooo_path.get()
        if not prg and os.name != 'nt':

The "os.name != 'nt'" check is redundant, it's always true.

filename = filename.replace('"', '\\"')
if os.name == 'dos':
        filename = filename.replace('/', '\\')
        prg = prg.replace('/', '\\')
cmd = "\"%s\" \"%s\" &" % (prg, filename)
if os.name == 'dos':
        cmd = "\"%s\" \"%s\"" % (prg, filename)
self.status.config(text="Starting viewer...")
        res = os.system(cmd)
except UnicodeError:
        res = os.system(cmd)

This is not a correct way to escape filenames. Use subprocess.Popen to spawn external programs. Also, I have no idea was catching an exception and then re-running the very same code is supposed to achieve..

def removeXMLMarkup(self, s, replace_with_space):
        s = re.compile("<!--.*?-->", re.DOTALL).sub('', s)
        repl = ''
        if replace_with_space:
                repl = ' '
        s = re.compile("<[^>]*>", re.DOTALL).sub(repl, s)
        return s

This is not a correct way to parse XML. Use a real XML parser, e.g. xml.etree.

parts = re.split("\s+", query.strip())

This could be rewritten more readably as:
parts = query.split()

regex = re.compile(re.escape(query.lower()), re.DOTALL)
if regex.search(docstring):
        return 1

This is weird. Why re.DOTALL when there couldn't be any unescaped dot in the regular expression? Also why bother with compiling regular expression for a fixed string when it will be used only once? It could be more readable rewritten as:

if query.lower() in docstring:
         return 1

(There are more instances of very similar code in the same method...)

# TODO: are all OOo files utf-8?

No, as far as I can tell there is no such guarantee.

title_match = re.compile("<dc:title>(.*?)</dc:title>", re.DOTALL|re.IGNORECASE).search(docinfo)

Again, this is not a valid way to parse XML.

Jakub Wilk

Reply to: