Skip to content
This repository has been archived by the owner on Feb 26, 2021. It is now read-only.

Merge libp2p-websocket-star-rendezvous into this repo #73

Open
daviddias opened this issue Jul 20, 2019 · 9 comments
Open

Merge libp2p-websocket-star-rendezvous into this repo #73

daviddias opened this issue Jul 20, 2019 · 9 comments

Comments

@daviddias
Copy link
Member

Following on ipfs/js-ipfs#2222

@mkg20001
Copy link
Member

Unsure if that's a good idea dependency wise, since we'd be slamming the entirety of the socket.io-server (including uws, which is a native module) plus tons of other modules onto the machines of people who just want to use js-ipfs and the websocket-star client, thus wasting unnecessary disk space among other things.

Otherwise this would be a great idea

The question is rather why nobody has bothered yet to discontinue ws-star ^^ (https://github.com/libp2p/js-libp2p-stardust is still a thing and it's finished, besides some missing PRs, and has a ton less deps)

@daviddias
Copy link
Member Author

Unsure if that's a good idea dependency wise, since w....

@mkg20001 the server wouldn't get published to npm as a module. It would just live within this repo, minimizing maintaining two separate repos (docs, duplicated tests, etc)

The question is rather why nobody has bothered yet to discontinue ws-star ^^ (

It's a different question all together. Discontinuing means that there needs to be a transition plan. I'll leave that for the libp2p tech leading team to decide

@mkg20001
Copy link
Member

@mkg20001 the server wouldn't get published to npm as a module. It would just live within this repo, minimizing maintaining two separate repos (docs, duplicated tests, etc)

In that case we should move it there

@mkg20001
Copy link
Member

It's a different question all together. Discontinuing means that there needs to be a transition plan. I'll leave that for the libp2p tech leading team to decide

This has been discussed in #70 and I still haven't received a proper answer. It's been half a year now...

(I heard something along the lines as "publish tutorials", but this doesn't address the reason why I created stardust: To fix the many problems of ws-star)

Additionally nobody has helped me with this PR: libp2p/js-libp2p-crypto#125
I've given it up and I'm waiting for contributors to help

@mkg20001
Copy link
Member

In that case we should move it there

Should git history be kept, or should it only be added as is?
I'd go for the first, since history can be useful sometimes

@daviddias
Copy link
Member Author

agreed, we want to keep git history :)

@mkg20001
Copy link
Member

Did, according to https://stackoverflow.com/a/21353836/3990041

@daviddias
Copy link
Member Author

daviddias commented Jul 21, 2019

@mkg20001 I appreciate you taking the initiative, however, please do not commit and push to master, it significantly erodes the trust that is deposited in you for having push access. That permission level does not grant you (or anyone with it) the right to push to master.

Specially for this change which requires coordination with the Infra team to ensure that they know where to find the latest version of the server.

Let's do it right. Please:

  • Revert both the commits:
  • Submit them again was a Pull Request, wait for review.

//cc @jacobheun @vasco-santos

@daviddias daviddias reopened this Jul 21, 2019
@mkg20001
Copy link
Member

maciej@mkg-pc:~/mr/ws-star$ git log | head -n 10
commit 0a4775c024cf87378c6b716a0930c2cad4fb88e7
Author: Maciej Krüger <mkg20001@gmail.com>
Date:   Sun Jul 21 23:22:53 2019 +0200

    misc: revert server/ merge

commit 0cdd57d2382b0c4fe79d8cd2a7f66e882fe96e5d
Merge: c843e9d f72cfb1
Author: Maciej Krüger <mkg20001@gmail.com>
Date:   Sun Jul 21 12:53:03 2019 +0200
maciej@mkg-pc:~/mr/ws-star$ LC_ALL=C git merge f72cfb146d8b3fa6611ca134d04245fd8e96cfd4 --allow-unrelated-histories 
Already up to date.

I tried. I can't remerge them, since they are already part of the history.

I'd have to rebase the rendezvous repo against a new commit initial commit to change the commit ids.
I could do that, if wanted, yet it would likely bloat history by another 70 commits, since the history of rendezvous would be added twice.

The commits could also be removed by a force push.

Also, sorry, I've made a mistake, ack'd. I've made sure this won't happen again, using a git hook.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants