neojski / visualizeRevTree

Visualize couchdb/pouchdb revision tree right in your browser
MIT License
37 stars 6 forks source link

Decoupling the Rendering and the Logic #14

Open nadeeshaan opened 9 years ago

nadeeshaan commented 9 years ago

Hi guys, We are currently (my self and @robertkowalski) are involved in a project (http://www.google-melange.com/gsoc/proposal/review/student/google/gsoc2015/nadeeshaan/5673385510043648) in fauxton to visualize the revision history of a couchDb Document and allow navigation and other document manipulation features. In order to do so we are willing to use your project as the base. Since we use React and Flux architecture in Fauxton we cannot use the current project as it is. In order to integrate this project with Fauxton, logic and the rendering should be decoupled. We Would like to get some feedback about this idea and the possibility of doing so.

neojski commented 9 years ago

Hi @nadeeshaan. I will do my best to help you!

Generally, most of the logic is pretty easy and all it does is call draw function that draws the image using paths, deleted, winner and minUniq (that I use to draw shorter names).

I think I'm in favor of making this visualizeRevTree use npm and browserify in order to make it more modular. Would it work for you? I'd say, export a function called getRevTree or similar. I can also tweak the API so that it suits your needs.

robertkowalski commented 9 years ago

cool sounds great!

maybe we can do this work in two steps:

  1. make use of browserify, set everything up - then send a PR
  2. refactor / expose the functions we need

what do you think of refactoring it so that using pouchdb gets optional (you provide a callback for each db action)

what do you think of providing a callback for the drawer which could also get injected by callback

@nadeeshaan @neojski what do you think regarding these three points (steps, callbacks for db, callback for drawer)?

neojski commented 9 years ago

Steps look good.

However, I'm not sure how would make pouchdb dependency optional. All this code does is get the data from couchdb using pouchdb API and then draw it. How is it set up in Fauxton so that you don't want pouchdb dependency?

What do you mean by the drawer?

robertkowalski commented 9 years ago

The drawer would be the renderer / rendering

In Fauxton we are using backbone models/collections for talking to couchdb

On Mon, May 4, 2015 at 4:51 PM, Tomasz Kołodziejski < notifications@github.com> wrote:

Steps look good.

However, I'm not sure how would make pouchdb dependency optional. All this code does is get the data from couchdb using pouchdb API and then draw it. How is it set up in Fauxton so that you don't want pouchdb dependency?

What do you mean by the drawer?

— Reply to this email directly or view it on GitHub https://github.com/neojski/visualizeRevTree/issues/14#issuecomment-98737971 .

neojski commented 9 years ago

I did some initial work of browserifying stuff. I'm open to your suggestions about point 2. Also, I'm open to contributions!

robertkowalski commented 9 years ago

wow awesome!

@nadeeshaan do you think we can use it?

robertkowalski commented 9 years ago

@neojski maybe you already solved point two :)

let's wait for feedback from @nadeeshaan

neojski commented 9 years ago

Just let me know what else you need. I can also publish it on npm.

neojski commented 9 years ago

@nadeeshaan and @robertkowalski, any news? By the way, do you have any links to source code where you want to use it? I'd be happy to see how it's used :-)

nadeeshaan commented 9 years ago

@neojski sorry for the late reply, since I have mistakenly missed the notification. At the moment we have converted the visualization in to react by using your logic. Some of the implementation has been bit modified in order to match the React implementation and flux architecture. Also you can find the current implementation here. Also I have to tell you your implementation logic is pretty amazing and which made our work easier. :) https://github.com/apache/couchdb-fauxton/pull/433

neojski commented 9 years ago

Oh, I see. It's a shame I couldn't make this code modular enough and you basically had to copy-paste it all. By the way, I'd be happy if I were mentioned somewhere as the original author of that code :smile: (or like contributor). How does it work in fauxton @robertkowalski, @nadeeshaan?

Before merging your project, please have another look as how we can improve on that. In particular, I suppose that visualization of the revision tree in fauxton will be maintained more than my project. I think that after this is fully copied into fauxton I can just start pointing to fauxton directly and leave a couple of screenshots here. What do you think guys?

nadeeshaan commented 9 years ago

@neojski we are definitely mentioning you as the owner of the core logic :) . We still did not do so since we are not in a merging state. IMO linking fauxton directly here as you have mentioned will be really, really nice. @robertkowalski I think that is a good idea, would like to have your opinion also. @neojski This is how the tree view looks at the moment. rev_tree

neojski commented 9 years ago

Sounds great and looks really nice. Please ping me once you're ready so I can have a look and do the updates here.

robertkowalski commented 9 years ago

Hi @neojski

we are working on this project as part of the Google Summer of Code and of course we will add you to our NOTICE and LICENSE file, and also add a few comments in the source!

The main issue that we can't use it right now is that that a build including pouch is too heavy for us (we don't use pouch anywhere in our project, mainly because pouch did not exist when the project started).

I am quite busy these days, but it still bugs me a lot that we can't reuse at least parts of the code, so stay tuned. Thanks for your support so far, maybe @nadeeshaan has time to make PouchDB / data-fetching in general modular and decouples it and sends you a PR.

I think as we want to use your code we should work on the revtree project - I was quite surprised (in a positive way) that you already did a lot of the main work for us - thank you again!

@nadeeshaan is decoupling the data fetching something you could work on next week? I would suggest a callback / callbacks that you pass in that returns the data in the current format the jquery ajax and pouchdb calls return. Then this project uses a main entry file that still uses pouch and jquery ajax and the entry file for fauxton is passing in backbone models or data from backbone models.

nadeeshaan commented 9 years ago

@neojski can you give me a brief introduction about the code base? I think you have included the browserify in the code base and would be a great help if you can give me how the execution flows works. Also how can we affect the changes changes we done in the source, without doing the changes in the index.js in dist folder :)

robertkowalski commented 9 years ago

@nadeeshaan please follow my advice that i gave you some days/weeks ago to learn what browserify is by visiting the official homepage and/or reading one of the countless tutorials on the web.

neojski commented 9 years ago

@nadeeshaan, once you read about browserify I'll answer the second part of the question.

So, basically, dist should not be included into the git tree. The only reason it is here is that I'm using gh-pages branch so that you can see the demo. Ideally (you can give it a try after you know a tiny bit how browserify works, I can recommend that as an exercise and submit a PR :-)) there should be a master branch with no dist and then a gh-pages branch with dist. Dist is automatically generated from source code by running npm run build.

nadeeshaan commented 9 years ago

@robertkowalski I followed the tutorials weeks ago to understand what browserify is and read some blog posts to get an idea about it. Thanks to you I could read the code base :). @neojski I will try to create a master branch and that is what I was wondering where is the pouchdb library and how it runs when I issues the build command early :) Ping you whenever I am stuck

nadeeshaan commented 9 years ago

@neojski I figured out the thing I previously asked. Without building time to time we can run the watchify to poll on the changes. I will send a master PR so it will be easy for the development @robertkowalski Since we decoupled the logic previously, in order to have our current rev tree PR I think we can you the same decoupling procedure?

neojski commented 9 years ago

@nadeeshaan, there's already npm run watch that does that. The only problem I don't know how to solve is building dist automatically and committing it to gh-pages branch. Apparently this can be done using travis.

neojski commented 9 years ago

How is your progress @nadeeshaan, @robertkowalski? Any news?

neojski commented 8 years ago

Ping @nadeeshaan, @robertkowalski.

robertkowalski commented 8 years ago

hi @neojski,

this task was part of nadeeshaan's google summer of code project. as the summer of code is over now and nadeeshaan does not continue to work on the task and is also not responding in this thread i would vote to close the issue.

sorry for any inconveniences!

neojski commented 8 years ago

@robertkowalski, how did the project go after all? Did you guys finish putting this into fauxton?

robertkowalski commented 8 years ago

@neojski nope, sadly not