Merge lp://staging/~gero-bare/wingpanel-indicator-datetime/24hs-12hs-system-setting into lp://staging/~wingpanel-devs/wingpanel-indicator-datetime/trunk

Proposed by Gero.Bare
Status: Merged
Merged at revision: 122
Proposed branch: lp://staging/~gero-bare/wingpanel-indicator-datetime/24hs-12hs-system-setting
Merge into: lp://staging/~wingpanel-devs/wingpanel-indicator-datetime/trunk
Diff against target: 88 lines (+53/-2)
3 files modified
src/CMakeLists.txt (+1/-0)
src/Services/ClockSettings.vala (+27/-0)
src/Widgets/PanelLabel.vala (+25/-2)
To merge this branch: bzr merge lp://staging/~gero-bare/wingpanel-indicator-datetime/24hs-12hs-system-setting
Reviewer Review Type Date Requested Status
WingPanel Devs Pending
Review via email: mp+289574@code.staging.launchpad.net

Commit message

Check for user's clock-format 24hs/12hs and set the panel's clock format according with that.

Description of the change

Check for user's clock-format 24hs/12hs and set the panel's clock format according with that.

To post a comment you must log in.
Revision history for this message
Danielle Foré (danrabbit) wrote :

You have two identical translator comments. Probably they should now call out that one is for 24hr and the other is for 12hr.

But do they need to be localized if we're checking the time format string here?

Revision history for this message
Danielle Foré (danrabbit) wrote :

sorry, I meant clock_format key

119. By Gero.Bare

Removing a comment, and reformulating the 24hs string. It shouldn't need to be translatable.

Revision history for this message
Gero.Bare (gero-bare) wrote :

@danrabbit Indeed the 24hs formant doesn't need to be located.

I reformatted the string, now is a normal one and removed one comment.

Thank you, I don't know what I was thinking.

Revision history for this message
kay van der Zander (kay20) wrote :

Looks good code wise. Only the license for ClockSettings.vala i think is wrong. Because there is no real "wingpanel developers" company. I would use "elementaey LLC"

Revision history for this message
kay van der Zander (kay20) wrote :

Sorry typo. elementary LLC

120. By Gero.Bare

Modify copyright notice.

Revision history for this message
Gero.Bare (gero-bare) wrote :

@kay20 Ok, no problemo.

Revision history for this message
kay van der Zander (kay20) wrote :

One more codestyle thing. See diff comment
Then i approve this branch ;)

121. By Gero.Bare

Use if/else instead of a hack.

Revision history for this message
Gero.Bare (gero-bare) wrote :

@kay20 Ok, done.

Revision history for this message
Danielle Foré (danrabbit) wrote :

Corentin had suggested it might be better to use %H for 24hr format. It sounds like that is the convention is to use a leading 0 with 24hrs

122. By Gero.Bare

Use %H (with leading zero) instead %k.

Revision history for this message
Gero.Bare (gero-bare) wrote :

@danrabbit Crap I got that wrong, and I even didn't test for it. Thanks.

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