search

SamAmco / track-and-graph

An android app for tracking personal data and creating custom graphs
GNU General Public License v3.0
438 stars 40 forks source link

Datatransformation: Add foundation #132

Closed hhpmmd closed 1 year ago

hhpmmd commented 3 years ago

Starts implementing #69

Hi. So i finally got around to start implementing the data transformation language + backend. It is far from being done, but I am really happy with the building blocks I got now and in general think it's a very solid foundation.

I am currently focusing mostly on tests, trying to make sure that everything behaves like it should and that extensions don't break everything. I also added a very rough fragment with an editor and some buttons to test out some things, but that is just for debug purposes (obviously). As mentioned in the issue discussion I think it would be best if someone with more UI knowledge implements the ui part. This is also why I wanted to share the foundations now, so that the UI work could be done in parallel. I also failed to get real data from the dao bc of threading stuff. But for now tests are best anyway.

There are also hundreds of small language decisions to be made (how to name functions, bunch of small things) for which I would be grateful for feedback. I guess I'll post specifics here or in #69 ? (maybe this github projects thing makes sense? i haven't used it yet)

I think I should make good progress on this in the following weeks. I guess some sort of beta testing for the language aspects would make sense. Would be a shame to get stuck in a backwards compatibility nightmare :)

There is lots to talk about, maybe I'll add some comments here later. If you have any questions feel free to ask

hhpmmd commented 3 years ago

When you check out the code, ignore the files in antlr.generated. They are automatically generated from the specified grammar.

hhpmmd commented 3 years ago

A chunk of the code is based on https://github.com/ftomassetti/LangSandbox which is licensed under Apatche-2.0. I added a license disclaimer to these files. To my (limited) understanding that should be all that's requested.

SamAmco commented 3 years ago

Wow you've been doing a lot of work! .. I'm going to need some significant time to sit down and review all this (as well as your other PR). I think you are right we're going to need a better system to manage and keep track of the work that's being done and discuss the issues remaining. I will try and take a look into github projects and see if i can set up a board and break down the workload a bit. I should be able to look into this stuff on the weekend so bear with me.

SamAmco commented 2 years ago

Ok this is tough to review in one go because it's very large. First thought is, can we remove the generated folder from git or do we need it in source control for some reason? Other than this what I have seen so far looks good but I need more time to look at it so i probably won't get through it this weekend.

SamAmco commented 2 years ago

Anything you can do to explain the code here and guide me where to start would be helpful if you have time.

SamAmco commented 2 years ago

I tried to create a project board and add you to it. Let me know if you can see it. I don't have time yet to break down more tickets etc but feel free to add notes and tickets there as relevant.

hhpmmd commented 2 years ago

First thought is, can we remove the generated folder from git or do we need it in source control for some reason?

Ok, I found a way to tell gradle to generate the files before building. Only thing is that now when somebody clones the repo they will get unresolved references from android studio, and they need to build to generate these files. This might cause some confusion and should maybe be mentioned somewhere in the readme? On the other hand clean rebuilding the project with gradle should be the goto for unresolved references anyway.

hhpmmd commented 2 years ago

I tried to create a project board and add you to it. Let me know if you can see it. I don't have time yet to break down more tickets etc but feel free to add notes and tickets there as relevant.

I can view the board, but I can't interact with it (or figure out how to do it). But this might be because it is currently set to closed.

SamAmco commented 2 years ago

I tried to create a project board and add you to it. Let me know if you can see it. I don't have time yet to break down more tickets etc but feel free to add notes and tickets there as relevant.

I can view the board, but I can't interact with it (or figure out how to do it). But this might be because it is currently set to closed.

I suspect you're looking at the wrong board. I created two while experimenting. Try here: https://github.com/users/SamAmco/projects/1

SamAmco commented 2 years ago

First thought is, can we remove the generated folder from git or do we need it in source control for some reason?

Ok, I found a way to tell gradle to generate the files before building. Only thing is that now when somebody clones the repo they will get unresolved references from android studio, and they need to build to generate these files. This might cause some confusion and should maybe be mentioned somewhere in the readme? On the other hand clean rebuilding the project with gradle should be the goto for unresolved references anyway.

That's ok, in my experience lots of projects have generated file references that require building.

hhpmmd commented 2 years ago

Ok I can interact with the board now. I see that this one is not bound to the repo but rather to your account.

I assume that's so we can have a separate repo for the data-transformation "issues"/discussion? That seems to me like the best solution with regards to the tools github has given us. I mean there still are some drawbacks etc but we can definitely give it a try.

Because I wouldn't want to clog up the issues here with the ~15 potential things to discuss etc.

SamAmco commented 2 years ago

So the reason why it's linked to my account rather than the repo is that that was the only way I could see to do it that would enable me to add you in as a collaborator. I think perhaps you should try creating issues on the board for the points you want to discuss. Once we have resolved a solution we will move them to done and create separate tickets for the actual implementation. How does that sound?

hhpmmd commented 2 years ago

Sure, but I can't create issues just linked to the board, they have to have a repo attached to them. I can only create these text-notes on the board directly and I don't think they are good for discussions.

hhpmmd commented 2 years ago

Ok I hope the comments (while they do not make the thread here more readable) help you figure out what's going on. They usually refer to the whole file, you best view them in the files changed tab.

SamAmco commented 2 years ago

Sure, but I can't create issues just linked to the board, they have to have a repo attached to them. I can only create these text-notes on the board directly and I don't think they are good for discussions.

Ok I'm not convinced that this is a good way to do this either. Do you have an atlassian account? I could create a trello board for us? Let me know your user and I will create and add you. Unless you have a better idea.

hhpmmd commented 2 years ago

I could create an account. Can you discuss things on trello these days? I haven't used it in almost 10 years i feel like.

Well that would exclude others from chiming in on discussions. But it's not that I would expect lots of feedback. We can go with trello.

Ok, account name is hhpmmd

SamAmco commented 2 years ago

Right sorry it took me a while but the board is up. You should be able to create tickets and we should be able to have conversations on those tickets as well as attach resources like screenshots etc.

SamAmco commented 2 years ago

Btw please let me know here when you're happy with this PR again and I'll take another quick look and merge if all ok.

SamAmco commented 2 years ago

I created a PR here: https://github.com/SamAmco/track-and-graph/pull/135

I'm not really sure how to add you as a reviewer to my PR's without adding you as a collaborator to the project which I'm reluctant to do just because it would give full write access directly to the repository and I'd rather we enforce that all changes must be merged via PR.

In the end I just added a comment to the ticket on trello referencing the PR and asking you to review it. Hope this is ok? Would be nice if I could notify you in some way? maybe trello did this when I tagged you?

SamAmco commented 2 years ago

Update on this. I've been looking at this again today. I think this PR is just far too large is the problem. Right now I'm thinking i'm going to basically start again and build up to something like what you have here and create several PR's as I go. There are some issues with the grammar for example and I would like to get the most basic grammar correct before moving on to implementing anything else. Things like the code editor should certainly be a separate PR. I also think we should eliminate as much grammar as possible for now. I don't see a need to support print, var etc yet. I think we can get rid of DECLIT and INTLIT and just have NUMBER. I think in the first pass we should probably get rid of all the aggregation and time grammar stuff. We can add these later to support the features that actually need them.

I think what you've done here is really valuable and will help a lot, but it's just a bit much at once.

SamAmco commented 2 years ago

Ok I started by creating a PR just for a very basic grammar: https://github.com/SamAmco/track-and-graph/pull/138

Keen to get your thoughts.