-
Notifications
You must be signed in to change notification settings - Fork 110
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
Bring revolution home? #2926
Comments
The aim is to achieve a self-contained suite of commands that could replace core commands, with the goal of having them in -core. ATM I see no benefit from doing that outside an extension, as -core has too much baggage that is/becomes irrelevant in the scope of this work. The more I work in this framework to more I appreciate the ability to draw a clear line wrt what kind of functionality I want to build on. Moreover, there should be nothing in -revolution that could be cherry-picked short of the whole thing (which is not ready). Small non-intrusive changes made their way into -core anyways and already. I do not plan to change this (i.e. fix in -core what needs fixing, but limit to just fixes). Another point that makes me prefer to status quo is the speed. I need to move fast ATM and with this setup I can have 2-3 times the changeset iterations that I can possibly achieve in -core in any given day, just due to the test suite runtime alone (not bringing up the direct mode issue again...). At the same time I can co-install this development functionality with a stable datalad installation (without having to switch), and the stuff that is written already gets a lot more real-world usage that it would get, if it would live in some long-lived branch (with the need for repeated merges and conflict resolution). That being said, I am very much open for alternative workflows that have the same benefit, but less of the disadvantages that a separate code base implies. The current approach is simply the best of the possibilities that I am aware of. |
Where such an alternative falls short? |
FTR: when datalad/datalad-revolution#29 is merged I would consider the revolution done. I see no path how to incorporate this into core without major friction. I consider Anyways.... the revolution usually eats its children... |
so revolution will be done and then what? i.e. what is the long term plan for it -- to just to be used as an extension (and thus just by a selected/enlightened/forced-to group of users)? What
or parents? |
I dont have plans. If core absorbs it, I am good. If it stays an extension, I am good too. The extensions that need this functionality are good too, either way. Major friction point: direct mode -- there is not even a trace of support. |
This is a process that is now ongoing, and likely to keep going for a while. But not an issue anymore! |
What is the problem?
What is your plan @mih about the https://github.com/datalad/datalad-revolution ? To keep it as a separate extension for eternity or bring pieces (or entirety) into datalad codebase for perspective future possibly API breaking release?
As I expressed myself, reviewing those changes is not easy and thus largely is not done. As I stated before, IMHO having them even in
master
branch (aiming e.g. for 0.11 release), while possibly still carrying out bugfixing in 0.10.x branch would be IMHO more preferable long term.edits to outline steps to see this issue resolved:
rf-revolution1
branch to contain initial changesrf-revolution1
. WiPrf-revolution1-3rd
branch) bring in revolution code base into the core under 3rd/datalad_revolution (TODO: tools/injest_revolution_code to be reused to suck in the changes)rf-revolution1
branch, to keep mergingrf-revolution1-3rd
)git mv 3rd/datalad_revolution/datalad_revolution datalad/revolution
(verify that tests pass, needs relative imports)datalad/revolution/
around the core, where needed prefixing withrev_
*repo.py
tocore_*repo.py
, while having revolution ones replace ours, while importing thecore_
onesrf-revolution1
intomaster
datalad.experimental.revolution
to be enabled by extensions before importing API etc?)The text was updated successfully, but these errors were encountered: