search

django-rea / rea-project

A way to discuss and collaborate on the django-rea organization umbrella
1 stars 0 forks source link

Splitting into separate apps II #1

Open XaviP opened 7 years ago

XaviP commented 7 years ago

I've opened this issue as a continuation of freedomcoop/valuenetwork#215

ghost commented 7 years ago

Imho I'd start simple. Forking DEEP (mainly because @bhaugen made optional all faircoins references), and follow more or less the roadmap you proposed @XaviP, first separating into independent files the models, within a models folder, then the views etc.

Afterwards I would upgrade to Django 1.11. Later on, splitting everything into their own apps. But in the same repository so eventually users download a pypi dependency as in django, the whole thing, and install the desired apps, but everything is installed by default.

I created the repository as @fosterlynn suggested on which we can start refactoring https://github.com/django-rea/nrp

I am pretty new to the GitHub organizations setup, let me know if you have any issues making changes to the repo @django-rea/django-rea team.

@XaviP I can take on the models refactoring, or at least an initial attempt, but I am also working on other stuff, you may prefer to work on it. Please let me know :)

XaviP commented 7 years ago

I agree with this roadmap. I'm not sure, but maybe this first step, separating files, can be integrated when ready into all repos and live sites (if this has any practical sense and doesn't break anything). As commented by @bhaugen to us, you, @escanda, were already thinking about modularizing the core, so I think you are in better disposition to do it. But I can help in any way you need. I upgraded ocp django to 1.8, so maybe I'm in better disposition to upgrade the new modularized core. Is this ok with you?

ghost commented 7 years ago

Yep, no problem.

One thing I would appreciate though is the team to supervise the work as I am doing so if I follow a mistaken road or you prefer a different way of doing something, please, it is commented asap so we can rectify :)

Also Bum was interested in participating, if you are in touch, please remember him to accept the GitHub invitation hehe :)

XaviP commented 7 years ago

Ok! I've advised @bum2 about it.

bhaugen commented 7 years ago

Forking DEEP (mainly because @bhaugen made optional all faircoins references), and follow more or less the roadmap you proposed @XaviP, first separating into independent files the models, within a models folder, then the views etc.

I agree with forking DEEP, but I think we need to make sure that the changes I made to the faircoin and freedom coop hacks will still work for freedom coop. We can do that after forking DEEP. I'll think about some tests.

In general, as we refactor, more tests would help a lot. When I refactored the value equation and income distribution code last year, I started by writing tests, because that is some complex code and I was afraid it would get out of control as I changed it. The tests, and making changes step by step and always testing, made it a lot safer and easier and I lost less sleep...

ghost commented 7 years ago

I agree with forking DEEP, but I think we need to make sure that the changes I made to the faircoin and freedom coop hacks will still work for freedom coop. We can do that after forking DEEP. I'll think about some tests.

The idea I had in mind is that eventually, NRP will just be another Pypi dependency in the different forks. A core made of reusable models and reusable views, and hooks on which forks will extend and just add to their requirements.txt or setup.py file. For instance faircoins can make transactions on signals emitted at the required moments by the core app instead of making them optional (one possible way of decoupling them following Django's philosophy). For this we could create a starter project on which forks could implement their core logic and UI.

In general, as we refactor, more tests would help a lot.

I can help with tests. I think it's the perfect way to get acquainted with the code base.

ghost commented 7 years ago

The initial approach is ready to be reviewed. I basically moved faircoin_nrp into its own repository. Renamed valuenetwork to django_rea to start with the new branding. Moved the work app and the specifics (settings.py, etc.) into a folder called ocp. And updated a few routes. The app works as it did, and the tests are passing. In theory nothing is broken for the time being.

This is a possible approach. All criticism welcome :)

bhaugen commented 7 years ago

@escanda I admire your courage. We need it. Unfortunately, @fosterlynn and I will probably not have a lot of time for review this weekend. Relatives visiting.

XaviP commented 7 years ago

Good!

What about account app?

ghost commented 7 years ago

I've compared old ocp db with the db generated by installation: it seems that all prefixes in tables are the same: valueaccounting, account, work_, etc...

This first refactor was mostly branding, and making possible to coexist in the same repo OCP and DEEP (ocp is in its own folder, deep could need another) so the table prefix scheme proposed by @bhaugen is not yet implemented.

I also need to talk more details with Numa on DEEP so we have our own folder with our custom membership form and so on, we need to sketch this "branding" part as well.

The next refactor due tomorrow will be splitting into separate files, models, and if time allows, views.

What about account app?

The idea is to use the standard made by pinax and extend it, if possible, so we keep up to date to the latest releases and so we can remove the folder from the repository. Not sure yet if possible. I'll give it a try once we have completed some more of the refactor. The reason it is not within the ocp folder is because it is common to both forks so I chose not to move it for the time being.

bhaugen commented 7 years ago

What about account app?

The idea is to use the standard made by pinax and extend it, if possible, so we keep up to date to the latest releases and so we can remove the folder from the repository. Not sure yet if possible.

You'd need to look through the changes we've made to it. https://github.com/django-rea/nrp/commits/master/account

A lot of them were about making it work with the OCP work app, which might also apply to using it within any other app that has its own UI. Could probably also be different ways to accomplish the same goals...?

ghost commented 7 years ago

Could probably also be different ways to accomplish the same goals...?

Perhaps hehe. In the latest releases they improved support to plug stuff in and ultimately we'd fork them and make a pull request with more extension points.

Imho that effort may be better to be done after we have the django_rea better modularized, until then, the account app serves us well :) But it's something we have to do before upgrading Django.

Btw, whenever you have some time check out the models-refactor branch please. It can be done better, but I'd like your first to review it to see if it's a road you want the project to take or if we can take on a different approach.

bhaugen commented 7 years ago

See also https://github.com/django-rea/nrp/issues/1

fosterlynn commented 7 years ago

I'm going to try to summarize the telegram chat from today:

We have concerns about getting too far ahead in the splitting / refactoring while OCP continues to move ahead. And there is particular concern around the upcoming hackathon.

We decided to go ahead with the DEEP source for the splitting. All work will be done in branches so that it will be easier to merge in the OCP work when it is ready. The branches will not be merged until we plan how to merge OCP work, and probably will be merged after.

@bhaugen will write up his hacks in OCP so they can be addressed. In general, hard-coded names and currency interfaces will be made configurable.

In general, we are all committed to making the refactored repo work for all 3 parties involved.

Is this correct? @XaviP @escanda @bum2