Swarm POC3 #17041
Swarm POC3 #17041
Conversation
There was a problem hiding this comment.
Really sad to see there is so much change condensed into one single commit.
This can really make it hard in the future to work with the code when you e.g. want to get context for a certain line in the code. When the code is added in more commits you can w.g. do a git blame and see the commit where it was introduced that ideally gives some context (e.g. a issue). By condensing it to one single commit you loose this for a large portion for the code now and you will often hit this single commit now when doing so which does not give you as much context as the single commits would.
I know you linked to the original commits in the PR - but you add extra steps to the process this way - and especially when the code mutates it will get more and more unpleasant to work with.
Don't get me wrong - not asking to change this now - but we should really think about the process and perhaps prevent the same thing to happen in the future.
|
@ligi I strongly agree with you, and we should definitely not do this again. We plan to do small merges regularly from now. But keeping the original commits was not possible because there were hundreds of them, and they did not keep to the commit conventions needed by gitcop. And we would squash it on merge, so the original commits would be lost on master anyway. The original https://github.com/ethersphere/go-ethereum/tree/swarm-network-rewrite branch will keep the commits for future reference. |
| Sergii Bomko [ledgerleopard.com] | ||
| Domino Valdano | ||
| Rafael Matias | ||
| Coogan Brennan |
There was a problem hiding this comment.
Any particular reason for keeping a separate AUTHORS file? We already have one for the entire repository, this seems a bit wasteful to duplicate effort here.
There was a problem hiding this comment.
@karalabe the problem is that because we squashed the branch into one commit, the contributors that helped with POC3 will not get credited for their contributions as they usually would through git.
we'd still like to give them the credit, thus we added this file.
| │ ├── encryption ──────── @gbalint, @zelig, @nagydani | ||
| │ ├── mock ────────────── @janos | ||
| │ └── mru ─────────────── @nolash | ||
| └── testutil ────────────── @lmars |
There was a problem hiding this comment.
If you merged the contents of this file to .github/CODEOWNERS, the correct person also get a review request every time someone modifies these packages.
There was a problem hiding this comment.
Great idea, done in ethersphere/swarm@ababfd2
|
@ligi we took your comment deeply into consideration and as a result we are looking into integrating gitcop into the ethersphere repo with the same ruleset as in the main |
gbalint
force-pushed
the
swarm-network-rewrite-merge
branch
from
ababfd2 to
c5f85c8
Compare
gbalint
force-pushed
the
swarm-network-rewrite-merge
branch
from
c5f85c8 to
e187711
Compare
ghost
mentioned this pull request
The code submitted here is restricted to the swarm package.
POC3 is a result of more than a year of work and has been extensively tested, reviewed on the ethersphere fork https://github.com/ethersphere/go-ethereum
This PR has original commit history
ethersphere/swarm#171
swarm network rewrite project board https://github.com/orgs/ethersphere/projects/3