Skip to content

Refactorings to prep for a later show_seconds PR#213

Closed
FriendOfEntropy wants to merge 12 commits intoelementary:masterfrom
FriendOfEntropy:master
Closed

Refactorings to prep for a later show_seconds PR#213
FriendOfEntropy wants to merge 12 commits intoelementary:masterfrom
FriendOfEntropy:master

Conversation

@FriendOfEntropy
Copy link
Copy Markdown
Contributor

@FriendOfEntropy FriendOfEntropy commented Sep 7, 2018

Related to #208

Per discussion in a previously canceled show_seconds PR, this is a set of small refactorings to limit the amount of simultaneous changes for review when a new show_seconds PR is created later.

  • Parse_time will become especially complex and busy when second support is added, so making it a bit more succinct here.
  • Going ahead and adding a with_format creation version of TimePicker to the demo view to help with regression testing, demonstrate the usage in code. When show_seconds is added later, there will be a with_seconds equivalent of this being demonstrated

Copy link
Copy Markdown
Collaborator

@donadigo donadigo left a comment

Choose a reason for hiding this comment

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

Can you explain why the timepicker_with_format was added to the demo? It displays the same thing for me as the time_format. Is this something for the refactor?

@jeremypw jeremypw requested a review from danirabbit April 28, 2021 19:35
@jeremypw
Copy link
Copy Markdown
Contributor

@danrabbit This is related to an issue you raised, so I requested a review from you. Not sure if the author is still available to work on it.

@jeremypw
Copy link
Copy Markdown
Contributor

I am going to close this as the author has shown little or no activity since beginning of 2020 and no review has been forthcoming.

@jeremypw jeremypw closed this Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants