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

WIP Frontend Refactor #34

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

WIP Frontend Refactor #34

wants to merge 2 commits into from

Conversation

giulioz
Copy link
Collaborator

@giulioz giulioz commented Sep 8, 2020

Closes #32

@giulioz giulioz self-assigned this Sep 8, 2020
@nicomazz
Copy link
Collaborator

Thanks for this huge pr!
If time allows, would be better to split it into several few smaller ones:

  • One for the formatting changes to non-code related files
  • One with only the changes to use typescript (modifying only one js file, to make sure it works)
  • One on top of the last one, with all the other changes

If you don't have much time, then just split the random formatting changes from the js->tsx refactor plz.

Ideally would be better to make the changes to js files as a rename, and not as a deletion and addition. According to this the best strategy is to first rename them in a commit and then apply the content changes.

If you don't have time for the things above, just make the tests pass and I will try to do the rest :)

@giulioz
Copy link
Collaborator Author

giulioz commented Sep 15, 2020

Actually I've done a single commit because I wanted to make it at least build, I don't like broken commits. Unfortunately it was pretty difficult to move between compiling states without making a mess.

If you want splitted commits I can do it, but it's still a WIP, I wanted to fix more things before merging the PR. Right now the frontend code it's in a state of insanity to be honest (and it was even worse before 😅).

The test are failing apparently because of a python server error that I didn't touch, can you give me any help?

@nicomazz
Copy link
Collaborator

nicomazz commented Sep 15, 2020

Does it "compile" for you?

@giulioz
Copy link
Collaborator Author

giulioz commented Sep 16, 2020

image

I've just added serve for production file serving and it works fine form me.

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.

Convert everything in typescript
2 participants