orbitdb-archive / orbit

A distributed, serverless, peer-to-peer chat application on IPFS
MIT License
1.64k stars 117 forks source link

Notes: Contributing to Orbit #220

Closed daviddias closed 3 years ago

daviddias commented 7 years ago

I'm looking forward to be able to contribute to orbit more and also be more proactive at testing new js-ipfs stuff in orbit. I created this issue to ask questions of how the project is structured and also to collect my notes as I learn.

Will break notes in different comments to make them easy to link to and be actionable.

daviddias commented 7 years ago

Currently there are no tests, is this expected?

ยป npm test

> orbit@3.2.0 test /Users/imp/code/orbit
> $(npm bin)/mocha

  Orbit
    โœ“ TODO

  1 passing (10ms)
daviddias commented 7 years ago

I see that you have raised your fd limit when running in electron -- https://github.com/haadcode/orbit/blob/master/package.json#L27-L28 -- the fd issue has been fixed since June 2016, do you still hit it sometimes? Would be good to inform @whyrusleeping and @kubuxu about it

daviddias commented 7 years ago

What is the main different between 'orbit' and 'OrbitClient'? Is that OrbitClient is the actual application, while Orbit is just a thing to spawn in electron?

If that so, would it make more sense to just have one package.json that has npm scripts to spawn both?

I also believe that calling something a Client is confusion, unless it is just a Client UI to the Orbit core, but I don't believe that is the case right now as OrbitClient has the full thing on it. Clarify please.

I've also noted that there are deps repeated in both package.jsons with different versions, is this desired?

daviddias commented 7 years ago

The instructions to run orbit in Electron and in the Browser in the README are the same

image

https://github.com/haadcode/orbit#desktop

daviddias commented 7 years ago

The getting started link in the README.md leads to nowhere, but opening the API.md file I could find it, but found -- https://github.com/haadcode/orbit/blob/master/API.md#getting-started -- this:

image

Which signals me that it might be out of date. What does the _ mean? Why does this version -- https://www.npmjs.com/package/orbit_ -- exist?

daviddias commented 7 years ago

Humble request: can we make the code be StandardJS compliant? pretty please ๐Ÿ™๐Ÿฝ

daviddias commented 7 years ago

Important: I don't see where the ipfs-api is used from the Node.js process of electron, I only see it being used inside the browser process, both in the Electron and Browser versions, could you clarify?

haadcode commented 7 years ago

Thank you @diasdavid! These are fantastic notes to understand where we need to improve :)

I'll come back to you with details to all of these in the following days.

Quickly though: ipfs is spawned in Electron at https://github.com/haadcode/orbit/blob/master/index.js#L147 which then gets passed via IPC to the UI https://github.com/haadcode/orbit/blob/master/client/src/stores/IpfsDaemonStore.js#L42. ipfs-daemon uses js-ipfs-api.

The directory client/ is the web app, which needs to be moved to its own repo: https://github.com/haadcode/orbit/issues/112. Orbit itself was recently finally moved to its own module, https://github.com/haadcode/orbit/issues/111, and is now used from orbit_ package (_ due to to orbit not being available on npm). The entry point is here: https://github.com/haadcode/orbit/blob/master/client/src/stores/OrbitStore.js#L3.

Hope this gets you a bit further.

theobat commented 7 years ago

Hey @diasdavid wouldn't it be better to group the questions into one ordered post for beginners to find more easily the mapping between questions and answers ? (these questions are a good entry point for new developers to come)

daviddias commented 7 years ago

aaaah ๐Ÿ’ก So, in fact you spawn ipfs-api in the Node.js process, but you call it from the browser process in https://github.com/haadcode/orbit/blob/master/client/src/stores/IpfsDaemonStore.js#L44, because electron gives you for free an RPC for the functions avaialble in the other object, that is why you never got issues with the http requests in the browser.

Woa, finding out this one was a ride. This means I really don't have to do anything with regards to our plan of running ipfs-api in the Main process. It would be good if all of these was more obvious though.

@theobat true, I do hope that all of these questions get an answer that is part of the documentation, rather than in a group of issues. Let's see how it develops :)

daviddias commented 7 years ago

@haadcode so the goal is to have:

repos:

Giving that orbit-electron and orbit-browser will use orbit-ui and inject their ipfs instance? If that is the best option and plan, let's do it :D

haadcode commented 7 years ago

so the goal is to have ...

The goal is to have:

Note that the UI, ie. the browser version, is smart in that it checks if it's running in Electron and if not, it uses everything as you would in a browser with js-ipfs and orbit.

As a side note, also note that orbit_ package is in repo https://github.com/haadcode/orbit-core.

So: orbit-app (currently in client/) --> js-ipfs (via ipfs-daemon) + orbit_ orbit-electron (currently this repo) --> js-ipfs-api (via ipfs-daemon) + orbit-app

Does that make sense?

haadcode commented 7 years ago

This means I really don't have to do anything with regards to our plan of running ipfs-api in the Main process.

I believe so, yes.

It would be good if all of these was more obvious though.

I hope I will be able to (finally) split all these this week or next week.

Since this is the first time we really discuss this plan, @theobat any thoughts?

theobat commented 7 years ago

orbit-app (which is the UI, ie. browser version, uses js-ipfs via ipfs-daemon) orbit-electron (uses orbit-app and js-ipfs-api/go-ipfs via ipfs-daemon)

yeah sounds good ! So the current orbit repo disappears plain and simple ? Where should we put explanations for the structure though ? it needs to be somewhere visible ..

haadcode commented 7 years ago

So the current orbit repo disappears plain and simple ?

Not sure if we should kill this repo. I feel we might lose plenty of inbound traffic if we kill it since it's been here for a long time and linked in a lot of places.

How about we keep this repo as the landing page for all those repos, and in the future collect everything Orbit related here? Eg. there's orbit-textui (terminal client), orbit-bot, and hopefully in the future we have other language implementations too, etc. We could link to every Orbit project from here. Or we could have this repo to be the orbit-core module and link to the clients (UIs) and other related projects from here. Thoughts?

haadcode commented 7 years ago

Humble request: can we make the code be StandardJS compliant? pretty please ๐Ÿ™๐Ÿฝ

(with the "fiiiine" voice) Fiiiine. Let's do it step by step as we move the orbit repos to use aegir.

Mark the date @diasdavid :)

theobat commented 7 years ago

How about we keep this repo as the landing page for all those repos, and in the future collect everything Orbit related here? Eg. there's orbit-textui (terminal client), orbit-bot, and hopefully in the future we have other language implementations too, etc. We could link to every Orbit project from here. Or we could have this repo to be the orbit-core module and link to the clients (UIs) and other related projects from here.

Yes sounds like other ipfs repos. It's probably a good idea (although I did not look at the ipfs/ipfs repo until very recently I do believe it's a good idea to keep an "entry point" repo without much code)

daviddias commented 7 years ago

All right :D

So, quick list of tasks to tackle first:

haadcode commented 7 years ago

I'm gonna start reorganizing this repo. Having thought about it, I think instead of orbit-app we should name it orbit-web because eventually we'll have many "apps" for different platforms, eg. orbit-android-app. orbit-web would make it clear that it's a web (browser) app imo.

Are you ok with that name?

dignifiedquire commented 7 years ago

I like orbit-web makes it much clearer

daviddias commented 7 years ago

๐Ÿ‘ orbit-web

RichardLitt commented 7 years ago

I can help! @diasdavid your suggestions look good to me.

RichardLitt commented 3 years ago

I think this can probably be closed now. Thanks, @daviddias. Hope you're doing well. :)