Hi,
Kevin C. Krinke wrote (31 Oct 2013 20:23:44 GMT) :
> Tails Clock has evolved quite a bit in the last week, below is a
> summary of the current state of the project.
Congrats!
> * List of configurable options:
> [...]
> * Display 12 hour clock (default: yes)
The default should depend on the current locale.
> * Debian package[1] now signed with my key[2]
I've not looked at the packaging yet, but I can do that if you wish.
Please make sure it's Lintian clean first, so that we don't waste
time on obvious mistakes:
lintian --info --display-info --pedantic --color auto /tmp/buildd/*.changes
> * Non-tails-included dependency is still just python-tz
I certainly wouldn't mind adding a dependency on python-yaml, rather
than seeing yet another very partial YAML parser appear on
this planet.
> * Translations? (used google translate so far for strings)
IMHO, no translation is better than crappy automated translations:
it clearly indicates that there's work in need to be done.
> * Preferences dialog is very bare-bones looking
> * About dialog is even more bare-bones than prefs dialog
I suggest simply stealing GNOME3's design.
> General thoughts? Concerns? Addition TODO suggestions? Anyone?
It would be great for reviewers to avoid commits such as e971d8c that
contain both the change documented in the commit message, and
unrelated changes (in this specific case, removal of commented-out
code).
I think it's generally not seen as best practice to ship all these
autotools files and symlinks in Git: shipping autogen.sh, Makefile.am,
src/Makefile.am and configure.in should be enough. Especially,
symlinks that depend on a specific version of automake are not
too helpful.
The docstring on top of TailsClockApplet.py points to the obsolete
config file path.
Tails is GPL-3+. How about making this applet the same?
Writing to a fixed-name file in a world-writable directory
(/tmp/tailsclockapplet.log) is a no-no, even in debug mode.
Surely Python has some mktemp implementation.
> self.set_size_request(280,280)
> self.set_resizable(False)
I'm not sure this plays nicely e.g. with large fonts (accessibility)
and l10n (longer strings, sometimes). At least make it resizable, even
if you don't manage to get dynamic sizing work? It's frustrating as
a user to get a non-resizable window, with content that doesn't fit
in. If you have a hard time dealing with this, perhaps Glade can help
(yeah, I know, it's a pain to get a Glade version that supports GTK2
these days).
> def toggled_12hr(self,widget):
> self.update_general()
> pass
> def toggled_sec(self,widget):
> self.update_general()
> pass
> def toggled_tz(self,widget):
> self.update_general()
> pass
> def toggled_yr(self,widget):
> self.update_general()
> pass
Perhaps a tiny bit of meta-programming to avoid the copy'n'paste?
I'm unsure about the Calendar feature. I do like this feature, but
perhaps it can live only in the devel branch until it's polished?
I'm pretty sure pylint would mind the 140-columns lines one can see
here and there.
Thanks for supporting both GTK2 and GTK3, this is very helpful at this
point of Tails development :)
Congrats, too, for all the useful "#Translators:" comments.
I'm excited, and I do hope this can make it for Tails 0.22 or 0.23 :)
(Disclaimer: I've not run the code yet, sorry.)
Cheers,
--
intrigeri
| GnuPG key @ https://gaffer.ptitcanardnoir.org/intrigeri/intrigeri.asc
| OTR fingerprint @ https://gaffer.ptitcanardnoir.org/intrigeri/otr.asc
