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

Splitting into separate apps II #1

Open
XaviP opened this issue Feb 25, 2017 · 14 comments
Open

Splitting into separate apps II #1

XaviP opened this issue Feb 25, 2017 · 14 comments

Comments

@XaviP
Copy link
Contributor

XaviP commented Feb 25, 2017

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

@ghost
Copy link

ghost commented Feb 25, 2017

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
Copy link
Contributor Author

XaviP commented Feb 25, 2017

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
Copy link

ghost commented Feb 25, 2017

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
Copy link
Contributor Author

XaviP commented Feb 25, 2017

Ok! I've advised @bum2 about it.

@bhaugen
Copy link
Contributor

bhaugen commented Feb 25, 2017

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
Copy link

ghost commented Feb 25, 2017

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
Copy link

ghost commented Feb 25, 2017

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
Copy link
Contributor

bhaugen commented Feb 25, 2017

@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
Copy link
Contributor Author

XaviP commented Feb 25, 2017

Good!

  • I've successfully installed, following this guide (not the faircoin part)
  • 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...
  • All tests passing by running: ./manage.py test django_rea.valueaccounting.tests

What about account app?

@ghost
Copy link

ghost commented Feb 25, 2017

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
Copy link
Contributor

bhaugen commented Feb 26, 2017

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
Copy link

ghost commented Feb 28, 2017

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
Copy link
Contributor

bhaugen commented Feb 28, 2017

See also django-rea/nrp#1

@fosterlynn
Copy link

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

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

No branches or pull requests

3 participants