Codebase cleanup ideas

[jtwaleson]

Here are some ideas I have to make the codebase a bit better. This is just a brain dump.

1. Create a makefile with the common actions (running locally, linting, testing, etc)
2. Introduce python type checks to all python code ( https://docs.python.org/3/library/typing.html ) and mypy for testing it in CI/CD
3. Create top-level directories frontend/backend/docs/website to organize the content a bit
4. Introduce docker compose to split off the frontend and backend building steps.
5. Remove all compiled and/or vendor software. Those should could be built in github actions / heroku deployment.
6. Clean up the translation tooling, the github. Improve the translation tooling. It is an inherently complicated problem but this needs some investigation and see where we can optimize it.
7. Fix workflows committing back to the repository, that should not happen this is a bit of an unusual anti-pattern in my experience, I would like to investigate why exactly we do this and either document the reason or refactor it to achieve the same goals in a "cleaner" way.
8. Find all orphaned files and remove them from the repo.
9. Introduce git lfs to track binary files and do a history rewrite so the repo size can be made smaller again.

Note: updated some wording marked with strikethrough after @Felienne 's comments below.

[Felienne]

Lovely ideas @jtwaleson! I am super glad someone is taking ownership of the repo quality a bit more!

Before diving into the details, I think it is a good idea to do this from the first principles of the code base. I know they are not documented anywhere so let's place them here for now so they are explicit:

• People with little software development should be able to contribute, especially undergrad students, because they often do. So things have to be simple even if you have never done github or done professional development. This matters for things like makefile, those are most likely not known to them, so if we use them, it must be clear how to do that. This is also the reason we have a maybe silly script called "feed dev db" that just copies data over, because running something is easier than copying.

• We want Python devs with no front-end experience to be able to work on the back-end with ease. That is the reason we commit the generated javascript to the repo, so that you don't need npm to test locally with the latest version of the front-end.

• Another third principle that we have happen to have documented is to be nice and respectful of the current state of things (see the readme) Phrasing like "this should be" or "this should not happen" are not the way we talk to each other here. Very often there were decisions made that you were not aware of, or trade offs considered (see the points above). Being inquisitive is the way to success in this project (and I imagine in other places in the world too), for example "why do we..." or "could we also....".

• Finally, cleaning up the code base I think serve clear goals. We have decided earlier in contributor meetings that we want to focus on the ease with which people can contribute to the code base, and definitely a lot of your points do that (1 and 3 for example). Another good goal is to make stuff less error-prone for users (2 would likely find and prevent bugs). In going forward maybe it is good to discuss what goals these and other ideas serve. For point 9 for example, I see how it is better theoretically (and probably not a lot of work) but I don't immediately see how it significantly improves developer experiences. But maybe I am overlooking things! That's why it is nice to motivate the ideas, that will also help prioritize.

On to the points:
1, 2, 3, 4, 8 and 9 seem like great ideas to me (taking into account the points above, it should be understandable to novices). We could decide in a meeting what to prioritize in terms of impact for developers or users.

For 5 and 7 we have clear motivations so I'd prefer to leave these. The hashes workshop is the way it is for a few intricate reasons and we have benefitted a lot from it being like that and since it is an important guard for quality, I fear changing it. However, I have noticed thanks to this discussion that the hashes workflow commits everyday (it initially ran weekly and then was not sooo overwhelming for the commit history) I will pick that up and make it run a bit differently, such that it only commits when it needs to limiting future commits, so thanks for pointing that out.

For point 6 I am not sure what you mean, but a lof of Weblate stuff could surely be improved! Note though that weblate (however much we hate it and its annoying PRs and commits) has been a core part of the success of Hedy, meaning that some things will remain the way they are.

Hope this helps and looking forward to chat about this more!

[jtwaleson]

Great! Thanks for your elaboration @Felienne. I was rushing to an appointment so I missed some nuance and didn't finish point 6...

I completely agree on the way of writing improvement suggestions. I have updated the wording above and added point 6. Just to re-iterate: I wrote these as my own mental notes and request for comments, definitely not to pass judgement. I'm an absolute newcomer to this repository and am not aware of how things work / should work yet.

I'm going to go ahead and do some more investigating and will probably create some PRs based on the points you agreed on.

[Felienne]

Ow and one more thing: It is hard for me to estimate how much work each of the points is! That too could be quite relevant in prioritising.

[jtwaleson]

1. Makefile -> 1hr
2. Introduce python type checks to all python code. -> 4hrs for some basic setup + CI/CD, another 8 or so to get to 80% type hint coverage.
3. Create dirs frontend/backend/docs/website. -> 2 hrs
4. Introduce docker compose. -> 2 hrs
5. Rm compiled and vendor software. -> 4 hrs
6. Improve translation tooling -> 12hrs
7. Fix workflows committing back to the repository. -> ??
8. Find all orphaned files and remove them from the repo. -> 2hrs
9. Introduce git lfs to track binary files and do a history rewrite so the repo size can be made smaller again. -> 1hr BUT we need to agree with all contributors on performing a history rewrite and the local setup will be more complicated for new contributors.

[jtwaleson]

These are very rough "shooting from the hip" estimates.

[jtwaleson]

10th point: use black for code autoformatting.

[jtwaleson]

I created #4453 for this but we decided to close the issue as we already have autopep8 of which I was not aware yet. Compared to autopep8, black is a bit more opinionated, faster and modern so we can decide to implement it later at some point. Right now it's not worth the migration and massive rewrite of the codebase and other points can be prioritized.

[jpelay]

Here are some ideas I have to make the codebase a bit better. This is just a brain dump.

2. Introduce python type checks to all python code ( https://docs.python.org/3/library/typing.html ) and mypy for testing it in CI/CD

This is a great idea that I've been considering for a while! Thank you for picking this up, I think type checks is the way to go forward.

6. Clean up the translation tooling, the github. Improve the translation tooling. It is an inherently complicated problem but this needs some investigation and see where we can optimize it.

Yes, the translation stuff is certainly a place where we could be a little better.

8. Find all orphaned files and remove them from the repo.

Great!

I love the idea of improving the quality of the code, and really appreciate someone taking care of it a bit for us.

[jpelay]

Hi! Something that came up to me, and it's been bugging me for a while: is there a way to enforce code standards on our typescript code? Something similar to black or autopep8? Our TypeScript code is kind of a mess on some places.

[jpelay]

Surre! In this PR it was introduced (#4224) and in the wiki you can read a bit more too: https://github.com/hedyorg/hedy/wiki#using-htmx-to-handle-interaction

[jtwaleson]

I gave prettier a try on the javascript/typescript code as well as the html files. Please take a look and let me know if this an actual pain point and worth pursuing @jpelay and @Felienne . It was super simple to get this up and running but would need some more work to create a proper PR.

See this branch on my fork: https://github.com/jtwaleson/hedy/tree/reformat-prettier

See this commit for the html files: jtwaleson@e8bea9a

See this commit for the JS/TS cleanup: jtwaleson@2b088b9

As always with introducing this kind of tooling: it's a trade-off between the massive change to the commit history but makes our lives easier afterwards.

Happy to discuss here or in the contributors meeting on Tuesday!

[jtwaleson]

Oh and to clarify: the prettier rules can be configured, these are just the default settings.

[Felienne]

Hi Jouke!

it's a trade-off between the massive change to the commit history but makes our lives easier afterwards.

Yes, and one more thing you might not have considered: it will also wreck all open pull requests, among which are also things we are still considering/working on. This is a reason why we, if we move this forward, had to time a potential merge carefully.

[jtwaleson]

Yes, agree 100%.

[jtwaleson]

11th point : move the /docs to the wiki and clarify in the README.md that the wiki should be used for all(?) documentation, as I believe that's the direction we want to go towards. The intro + architecture diagram on the main wiki page is really great :)

[Felienne]

11th point : move the /docs to the wiki and clarify in the README.md that the wiki should be used for all(?) documentation, as I believe that's the direction we want to go towards. The intro + architecture diagram on the main wiki page is really great :)

Yes, but contrary to your other points, this is one that we have been working on actively the last few months (top of the project board that we're currently working on)

[jtwaleson]

@Felienne excuse my ignorance but I couldn't find it. I searched for wiki https://github.com/orgs/hedyorg/projects/1/views/1?filterQuery=wiki here but didn't find anything with more details. I'm probably missing something silly.

[jpelay]

I think she means this: https://github.com/orgs/hedyorg/projects/1/views/5?pane=issue&itemId=25991510

[jtwaleson]

Great let's discuss at the next contributor meeting and see if we can get the task out of draft status.

[jtwaleson]

@Felienne for the docker compose / makefile / shipping vendor packages issues, I'd like to understand how the python devs with no front-end experience are currently working with the project. Do they work with docker / can we require them to install docker?

If we can standardize on a docker compose setup for local deployment:

• we can simplify the instructions and tooling a lot
• we can also handle npm building in a docker container
• developers would simply do docker compose up to run both the typescript compilation and python / flask execution

On the other hand, if we want the developers to work with a locally installed python (in a venv/virtualenv rather than docker) then I agree that the separate npm instructions would be too much effort to ask of novice python developers.

[jtwaleson]

I'm closing this discussion as the priority now is to resolve issues in the project board, not to get the perfect codebase. Along the way we will make some improvements, like we are already doing (most issues in this discussion fall under the "Contributor Happiness" label). For future reference: things like big migrations are better discussed during the contributors meeting to see what the right direction and priority are.