Merge lp://qastaging/~ijk/gtimelog/nutmeg into lp://qastaging/~gtimelog-dev/gtimelog/trunk

Proposed by ijk
Status: Merged
Merge reported by: Marius Gedminas
Merged at revision: not available
Proposed branch: lp://qastaging/~ijk/gtimelog/nutmeg
Merge into: lp://qastaging/~gtimelog-dev/gtimelog/trunk
Diff against target: 267 lines (+78/-39)
5 files modified
NEWS.txt (+4/-2)
README.txt (+12/-11)
gtimelog.desktop (+2/-2)
scripts/export-my-calendar.py (+4/-3)
src/gtimelog/main.py (+56/-21)
To merge this branch: bzr merge lp://qastaging/~ijk/gtimelog/nutmeg
Reviewer Review Type Date Requested Status
Marius Gedminas Needs Fixing
Review via email: mp+142036@code.qastaging.launchpad.net
To post a comment you must log in.
lp://qastaging/~ijk/gtimelog/nutmeg updated
246. By ijk

Use XDG Base Directory Spec

247. By ijk

Fix gtimelog.desktop validation. Use gtimelog icon instead of gnome-week.png.

Revision history for this message
Marius Gedminas (mgedmin) wrote :

This:

+ subject = 'Weekly report for {} (week {:0>2})'.format(who, week)

means a UnicodeEncodeError if who is a unicode string with non-ASCII characters.

Although testing reveals that currently `who` is a byte string, which causes bug 1117109, so this doesn't make anything worse.

(It also drops support for Python 2.6 or older, which is probably fine. We'll see if anyone complains about that.)

There are some minor PEP-8 transgressions that I'll fix up after merging.

(Ick, launchpad's review UI sucks. Why haven't I moved to github already?)

review: Approve
Revision history for this message
Marius Gedminas (mgedmin) wrote :

Actually, wait a second:

        xdg = os.environ.get('XDG_CONFIG_HOME')
        if xdg is not None:
            return os.path.expanduser(xdg)
        else:
            return os.path.expanduser("~/.config/gtimelog")

so if I set XDG_CONFIG_HOME=~/.config, then gtimelog will use ~/.config/gtimelogrc?

Shouldn't that be

        xdg = os.environ.get('XDG_CONFIG_HOME')
        if xdg is not None:
            return os.path.join(os.path.expanduser(xdg), 'gtimelog')
        else:
            return os.path.expanduser("~/.config/gtimelog")

and likewise for the data dir?

review: Needs Fixing
Revision history for this message
Marius Gedminas (mgedmin) wrote :

Also, should it really pass the value of the environment variable through os.path.expanduser()?

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches