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

swarm: Migrate p2p/simulations from go-ethereum #1805

Closed
wants to merge 1 commit into from

Conversation

chadsr
Copy link

@chadsr chadsr commented Sep 24, 2019

This PR coupled with this go-ethereum PR migrates p2p/simulations from the go-ethereum repository to here. (Technically it also migrates p2p/testing, but it is already duplicated in this repo)

Motivation for this came about from #1799, where that PR depended on a go-ethereum PR. @acud pointed out that it would in-fact make more sense to migrate p2p/simulations package to ethersphere/swarm.

I refrained from renaming the package, to avoid any confusion. I do feel like it could be refactored to somewhere else though (there are 3-4 simulation related packages scattered around), but I will leave that for core devs to decide. :)

@acud
Copy link
Member

acud commented Sep 25, 2019

@chadsr build is broken

@chadsr
Copy link
Author

chadsr commented Sep 25, 2019

@chadsr build is broken

@acud Yeah, apologies, my IDE bugged out yesterday and apparently let me down.

However I'm not sure if this PR is a good idea anymore? After finding some complications and what @fjl just added, i'm thinking it might be more appropriate that the packages stay in go-ethereum?

@chadsr
Copy link
Author

chadsr commented Sep 25, 2019

Going to close this, as it seems best to keep p2p/simulations in go-ethereum after all.

@chadsr chadsr closed this Sep 25, 2019
@zelig
Copy link
Member

zelig commented Oct 1, 2019

hmm, i thought it was a good idea, given noone is using this and we maintain it...

@chadsr
Copy link
Author

chadsr commented Oct 1, 2019

Reopening to facilitate some more discussion.
@zelig The package is used by one or two tests in go-ethereum. It seems fjl is keen on keeping the option to use it further in go-ethereum as well.

As an outsider, I see these options (but maybe there are more):

  • Continue the migration and use the swarm package as a dependency in go-ethereum.
  • Duplicate the package, so that swarm and go-ethereum maintain their own versions. Swarm version could then just be unified with the swarm specific simulation packages that already exist.
  • Keep it in go-ethereum and remain reliant on their release cycle.

@chadsr chadsr reopened this Oct 1, 2019
@acud
Copy link
Member

acud commented Oct 1, 2019

if go-ethereum are using this library then let's not migrate this. let's just PR stuff into go-ethereum and vendor it again. let's just make sure we are not dependent on them to approve this stuff - if we are the codeowners then let's maintain this code (or at least tell geth to vendor this from us so we can actively maintain it)

@acud
Copy link
Member

acud commented Oct 17, 2019

@nonsense @skylenet can I have your input on this? I'd rather we don't maintain duplicate code here and on go-ethereum, and I guess you two would be able to hook it up easily right?

@acud
Copy link
Member

acud commented Oct 20, 2019

@chadsr do we still need this PR? I think due to the fact you've submitted the go-ethereum PR this is no longer needed right?

@chadsr
Copy link
Author

chadsr commented Oct 20, 2019

@acud Only useful if others from the core team still want to give input, I guess. It's not needed in relation to anything else from me, so feel free to close it.

@acud acud closed this Oct 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants