Skip to content

Conversation

z1nc0r3
Copy link
Member

@z1nc0r3 z1nc0r3 commented Dec 18, 2022

#1678 Added hh:mm:ss tt to the clock formats to show seconds in the clock.

Copy link
Member

@jjw24 jjw24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi there, thanks for the pr.

As @onesounds mentioned, there maybe a slight impact on memory, could you please test how much impact there will be. This can be showing a screenshot of before and after using this format.

We will also make it clear to users that this may impact memory. @onesounds what's the best way to do this? Tooltip or warnin text etc?

jjw24
jjw24 previously requested changes Dec 18, 2022
Copy link
Member

@jjw24 jjw24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments please.

@jjw24 jjw24 added the enhancement New feature or request label Dec 18, 2022
@jjw24 jjw24 added this to the Future milestone Dec 18, 2022
@onesounds
Copy link
Contributor

I'll test and then if there are lot memory using, I'll add description part.(aka tooltip)

@onesounds
Copy link
Contributor

onesounds commented Dec 18, 2022

I have confirmed that resource usage does not increase in particular.
(Taoo and me have already checked the memory usage when inserting this feature.)
I think it would be okay not to add an explanation.

@jjw24 jjw24 added the review in progress Indicates that a review is in progress for this PR label Dec 20, 2022
@jjw24 jjw24 modified the milestones: Future, 1.11.0 Dec 20, 2022
@jjw24
Copy link
Member

jjw24 commented Dec 25, 2022

Haven't had a chance to take a look, will do so soon

Copy link
Member Author

@z1nc0r3 z1nc0r3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hh:mm:ss tt and HH:mm:ss tt are not the same.

@VictoriousRaptor
Copy link
Contributor

hh:mm:ss tt and HH:mm:ss tt are not the same.

I know, but tt in HH:mm:ss tt is redundant.

@jjw24 jjw24 removed the review in progress Indicates that a review is in progress for this PR label Dec 26, 2022
@VictoriousRaptor VictoriousRaptor merged commit 151c17a into Flow-Launcher:dev Dec 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants