Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Structure refactoring, splitting to smaller logical files #1

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

Edke
Copy link

@Edke Edke commented Aug 29, 2016

Hello @tbekolay.

I'd like to add some features to ti and this pull is a start as it's hard to maintain whole logic in single file. Therefore I separated code to more logic blocks. I also did minor Python 3.5 fixes.

More to follow, don't know if you will be interested:

  • moving data from json to sqlite as it's much simplier to collect and aggregate tracked times using plain SQL as doing processing in Python
  • adding some auto names while ti on like folder name, git branch etc (I'm really lazy person you know :) )

Feel free to comment, if this is direction you do not like, that is OK, just close this pull then.

@tbekolay
Copy link
Collaborator

tbekolay commented Aug 30, 2016

Hi @Edke, thanks for the PRs! I'll try to get to them sometime this week.

Splitting into separate files is all good, definitely makes sense. I'm not sure about the move to sqlite though, as having a plaintext backend that be tracked in version control is a core reason that I started using it in the first place. Despite simpler code, moving to sqlite means I can no longer look at the JSON file or analyze it without knowing the database schema, both of which are important to me. That said, we could make the storage medium configurable based on user preference, that seems fine to me. In any case, I'll have more concrete things to say when I look at this later this week 😄

@Edke
Copy link
Author

Edke commented Aug 30, 2016

I moved to sqlite for easier operations like aggregation. But I created
some sort of store interface, therefore making optional storage json vs.
sqlite shouldnt be a problem at all. We can definately sort these out to
suit both.

Ok, check out pulls, also git and write to me how we can proceed and make
proper release with new features and compatible with current version at the
same time.

@tbekolay
Copy link
Collaborator

tbekolay commented Sep 2, 2016

This PR looks to be getting a lot of other material in it, so I'm going to look at #2 first.

@Edke
Copy link
Author

Edke commented Sep 2, 2016

@tbekolay yes, sorry, this is a mess right now as after refactoring PR with new features against master would be confusing. If possible, try to look at final state of changes, then we can discuss how to proceed.

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.

2 participants