Write-For-Change / emaildrafter

An app to let people draft personalised emails for a good cause
GNU General Public License v3.0
10 stars 3 forks source link

Suggestion: Improve maintainability of the code #49

Open tjgurwara99 opened 4 years ago

tjgurwara99 commented 4 years ago

Hi guys,

I know I haven't been involved a lot with this project but I've been following this project for quite some time. I was wondering if you guys are considering adding a documentation of everything that you guys have done. It seems that this project has reached a level where it might be difficult for newcomers to collaborate.

This project has become (sort of) like a framework with its own features that could benefit from standard structure from a bigger framework than flask. I think the more CRUD based operations that will be added in this project the more there are risks with losing the ability to modify the database structure at a higher level and also lead to some potential problems down the line. This also leads to the main problem that the project would always need to be maintained by people who are already familiar with the database structure.

davidswarbrick commented 4 years ago

Hey, this is a really good point! The code as it currently stands is not super well documented, however I think/hope that in most cases we're using libraries etc as close to their intended use cases as possible, so that people familiar with Flask & Mongo will understand how things are being implemented. Your points about the database are especially valid, as, for example, the way one's development environment variables must be configured to access the databases is not documented on here.

It would be really helpful if you were able to elucidate how you think the best way would be for us to document the project? We could start a GitHub wiki, but from previous experience they seem to rapidly fall out of date with the current state of the project. Alternatively, we could extend the ReadMe to cover more details such as development strategies/naming standards/information flow/database setups, which would be more likely to be updated, however may be shorter and therefore less in depth. I'd hope that most contributors have written inline comments also, however of course there is always more background discussion than that presented and complex information flows can be hard to follow via comments.

Regarding your point about the CRUD operations, the reason we've gone with Flask over Django for example (as a framework with more features) was that in the beginning the app was incredibly simple and Flask sped up the process of turning it into a functioning site. We started with no database and later adopted Mongo because a member of the team was experienced with it. In the future we've been mulling ideas about REST, moderator pages & swapping databases etc. so I'd actually be very keen to discuss porting it to some other framework with more features and a more appropriate standard structure if that is what you are suggesting, however I'd also make the point that we've tried to emulate the way other frameworks approach databases (having an associated Python object which is created from data using some standard methods) so that our approach is hopefully not too alien or specific.

Thanks again for your interest in the project! If you think you'd like to get further involved I'd be very happy to answer any specific questions you may have, but am also especially keen to hear more about any barriers to people contributing that we could alleviate.

tjgurwara99 commented 4 years ago

It would be really helpful if you were able to elucidate how you think the best way would be for us to document the project?

There are many ways to go about this, but most of them depend on some technical decisions that need to be made.

We could start a GitHub wiki, but from previous experience they seem to rapidly fall out of date with the current state of the project.

Exactly, they get out of date very quickly. So I would also advise against GitHub Wiki too.

Alternatively, we could extend the ReadMe to cover more details such as development strategies/naming standards/information flow/database setups, which would be more likely to be updated, however may be shorter and therefore less in depth.

True, it would not be very useful for people if it is not very detailed. However, I was thinking of something entirely different to these approaches that only require people to add DocString comments in the code and use some documentation tool like Doxygen or Sphinx (the latter requires more manual stuff than Doxygen but is better looking in terms of design than Doxygen). We can create some GitHub Actions to generate the static HTML from Doxygen or Sphinx which will automatically be pushed to a branch in this repository called gh-pages for GitHub.io after merging a PR.

For an example, C-Plus-Plus is a repo that I follow a lot and they have a workflow which uses Doxygen to write documentation which you can find on their io page here.

Regarding your point about the CRUD operations, the reason we've gone with Flask over Django for example (as a framework with more features) was that in the beginning the app was incredibly simple and Flask sped up the process of turning it into a functioning site. We started with no database and later adopted Mongo because a member of the team was experienced with it. In the future we've been mulling ideas about REST, moderator pages & swapping databases etc. so I'd actually be very keen to discuss porting it to some other framework with more features and a more appropriate standard structure if that is what you are suggesting

Yeah, all of these features are supported by Django. Additionally it has support for multiple database management and a lot more powerful stuff.

however I'd also make the point that we've tried to emulate the way other frameworks approach databases (having an associated Python object which is created from data using some standard methods) so that our approach is hopefully not too alien or specific.

True, I wasn't worried about it being alien because it follows most python standards but I felt that it lacks checks that makes it error prone if anything it depends on changes (for example if EmailTemplate changes then it might not work as expected) and there are no fallback or preventative measure in place. However, this is my observation and I'm not very experienced with pymongo package so take it with a grain of salt😅

Thanks again for your interest in the project! If you think you'd like to get further involved I'd be very happy to answer any specific questions you may have, but am also especially keen to hear more about any barriers to people contributing that we could alleviate.

Thanks. I was thinking that since this is a student initiative and most people who will be managing this repository will be students who would more or less not have time to read all the internal workings of the code. So adopting a Django would help because of the restrictions placed by Django on how we can do something - granted it would be restrictive but restrictions make it more maintainable.