MyFinances
MyFinances copied to clipboard
TreyWW
cleanup: Code Cleanup | Open to all
Hey.
This project has grew, quite a lot, since I first started. It's taken slightly longer than I was expecting and I've in general made it more complex than it needed to be. Nevertheless - it continues!
But in order for this project to be approachable and maintainable, it needs a bit of a tidy up. Anyone that would like to help contribute, feel free to leave a comment here and make a PR when done. You don't need to clean up everything, there's no minimum or maximum limit. Clean up one function, or a file, or a html div, it's up to you!
This will be open for a while to help boost project approachability and for newcomers to get a better understanding of the projects codebase.
Thanks all, I appreciate any help given
SOME PARTS THAT STILL NEED REFACTORING:
- models.py (function names such as RandomCode, blank = True for orgs and users)
Hi @MuhammadHasnain77, since manage.py is a Django generated file I think it's okay to be left how it is. It's never going to be edited so I see no need for any improvements. And I don't want any functionality of it to be accidentally changed as it's the main file to run tasks.
Feel free to cleanup anything else though. There's python files in /backend!
Hi, I'm looking to clean up some of the views, will submit a PR when done.
Hey there @TreyWW. I'd like to help out. Should I list what I'm working on or just submit a PR when done?
Hi @rlanier-webdev. Up to you. Personally I'd wait as when I merge #87 in a few days that will have about 30 file changes.
We've also started using djlint for html files which is nice
If you do want to work on files, preferably work on like invoices or receipts until #87 merges if that's okay.
Sorry for the disruption
Hi! I would like to contribute to some cleanup/refactoring. I would like to refactor codes in /backend. Is there anything that I should know before I start? Thank you.
Hey @adnangif,
Not really. Make sure no functionality is changed that's too major, or make sure to document it in your PR if you do change anything. Once done make sure to run the below commands:
py manage.py test # this runs tests to make sure nothing has broke in the process
black ./ # this runs the black formatter that formats the files
Let me know if you've got any questions or need help as you go along. Thanks for your interest!
I've noticed that all the database models are written in a single lengthy file. Would it be acceptable for me to reorganize them by creating a 'models/' folder and separating each database model into its own file? I'll also ensure to update the corresponding imports accordingly.
Hey @NikolaSL00,
I guess we could move them to their own files, it just (in my opinion) makes it harder to find and navigate models. I've kind of put this off cause we don't have too many models, I simply collapse each one to only see the name until I need it. With their own module it'll have like 10 separate files making it a little harder to find each one.
We can try it though, feel free to move them into their own modules and we can see how it looks. Thanks :)
Hey,
I would like to help with /backend files. There is couple of nested and repetitive if statements that can be reduced and few except blocks that are to broad.
Hey @Domejko,
That would be great! I agree, there are definitely quite a few nested statements that can be better arranged.
Appreciate it! Let me know if you struggle with anything
Hey, I've talked with @NikolaSL00 and I will try to do what he mentioned in his comment.
Hi, I am trying to contribute to an open source project for the first time to gain some experience and I have only ever worked in a classroom setting, so I wanted to start on a task like this. I am unfamiliar with the .env file and settings. How much of this do I need to fill out in order to be able to make a simple code refactor/cleanup and make a PR?
Hey @vishalprashant, So the env file you can actually leave how it is. You can follow the documentation and setup SQLite and that’s the only real env variable you need. The docs will show you the rest
@TreyWW Thanks for the help. You mentioned blank = True at the top. In cases like the Invoice class in models.py why is blank = True even necessary? How does the functionality change if I just remove it from each variable and leave null = True to dictate what needs to be in those fields? Are all these fields not supposed to be mandatory?
Hello! I would love to contribute to the cleanup and refactoring task. Let me know if I need to know anything before starting.
Hey @devenderbutani21! One key thing I'd suggest is please, please, make smaller PRs rather than bigger ones. They often slow the whole process down, can take weeks and just clog things.
If you'd like to make drastic changes I'd prefer if you could ask first, so you don't waste time for me to just decline it.
Otherwise, make a few changes per PR, i'd rather more PRs but smaller, I can merge them in minutes rather than days.
Thanks! To get started you can use the docs
Okay! I will make smaller PRs for easier merge. Thanks for the heads up.
@TreyWW Thanks for the help. You mentioned blank = True at the top. In cases like the Invoice class in models.py why is blank = True even necessary? How does the functionality change if I just remove it from each variable and leave null = True to dictate what needs to be in those fields? Are all these fields not supposed to be mandatory?
Hey @vishalprashant,
Blank=True just fixes the admin page, otherwise it makes every field required. Any field with null we should set with blank
I also want to contribute and start my open source contribution journey.
Hi @TreyWW, I would like to start contributing to the cleaning of the code.
Hey @brainycarp67531, welcome to the project! Sure thing, what parts would you like to work on? Django Templates/HTML, Python, Service layer (would be helpful to have better structural ideas), developer documentation, anything like that in specific?
Thanks!
@TreyWW, I would like to start with python, but open to other areas also.
Sure thing. One core part of our new architecture is the Service layer. This sits between the backend request, and the response. I'd appreciate a second set of ideas to approach this problem, as I find my ways of doing things are a little... odd.
Give me a few minutes and I'll gather some more detail for you. Appreciate it!