mirror of
https://github.com/nspcc-dev/dbft.git
synced 2026-03-01 04:28:38 +00:00
Why dBFT requires NeoGO package #2
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @im-kulikov on GitHub (Nov 27, 2019).
It's strange to understand that the dBFT package requires NeoGo, which requires dBFT.
I propose to move dependencies into the dBFT package.
That adds the possibility to use the package separately in other projects without NeoGo requirements.
cc @fyrchik @realloc @anatoly-bogatyrev @roman-khimov
@roman-khimov commented on GitHub (Nov 27, 2019):
These are all modules, it makes no sense to shuffle them around. Just use modules from the origin repository that developed them.
I don't think this potential move would change anything.
@im-kulikov commented on GitHub (Nov 27, 2019):
That adds a possibility to ignore NeoGO mono repo as a dependency.
The dBFT used two types from NeoGO and only for that it requires it. That sounds strange.
@im-kulikov commented on GitHub (Nov 27, 2019):
My point is in moving or rewrites that types into dBFT and remove unnecessary dependencies.
@roman-khimov commented on GitHub (Nov 27, 2019):
And that gives us like... what, really? It's not a linux repository, it's like 18M repository.
Not so much, it reuses module that it needs to and that's it.
My point is that moving code between repositories completely destroys changes history and I just hate that.
And rewriting is like... please, don't. There is a module that implements exactly what you need, hoooray! just use it.
@im-kulikov commented on GitHub (Nov 27, 2019):
Nope. It adds a lot of unnecessary dependencies from NeoGO module.
For two types I should import 18MB? It sounds ugly.
@im-kulikov commented on GitHub (Nov 27, 2019):
It was assumed that part of the NeoGO package (pkg) will be allocated to separate repositories. Now we are turning NeoGO into a mono repo. What then was the point of putting dBFT as a separate repository, if it still requires NeoGO?
@roman-khimov commented on GitHub (Nov 27, 2019):
Like
iomodule? That's the only one being added. And to be fair it's only added to satisfy data type, no code should be pulled from it.That's git repository size, I don't think anyone cares about that. But you only really add one tiny module to the executable and that's it.
@roman-khimov commented on GitHub (Nov 27, 2019):
After discussion with @im-kulikov we've settled upon abstracting away this types with interfaces if there is a need to really separate this two repositories.
But looking at how these types are used (throughout the whole repository) it probably won't be that easy.
@im-kulikov commented on GitHub (Nov 27, 2019):
Yep. An interface would be a great decision.
@fyrchik commented on GitHub (Dec 2, 2019):
@im-kulikov to be able to use it from both
neo-goandneofs.@fyrchik commented on GitHub (Dec 2, 2019):
Well, we can probably hide
util.Uint*behind something likeinterface { Equal(..) bool }and require making wrappers in client, because function type can't be a subtype of another type.But
iopackage fromneo-gosimplifies marshalling a lot and makes it a lot more readable. It is used mostly for default implementations and we can get rid of it but then require client to implement all possible payloads. I'd rather prefer to be able to use a library that "just works" in simple cases and at the same time is rather flexible to be used in more picky situations.@im-kulikov commented on GitHub (Dec 2, 2019):
It's just syntactic sugar and I don't see any problems to use the binary package as is.
@im-kulikov commented on GitHub (Dec 2, 2019):
As we discuss with @roman-khimov, there is ok to use
Hasherinterface and returns the slice of bytes ([]byte). On the usage side, you can wrap it with an internal type as you want.@roman-khimov commented on GitHub (Dec 20, 2023):
I don't think we'll ever untie these.
@fyfyrchik commented on GitHub (Dec 21, 2023):
I have cleaned modcache and after this I have:
Makes me appreciate what Go team has done regarding versioning, though.
@roman-khimov commented on GitHub (Jan 30, 2024):
This can be useful for Bane, btw. As suggested in #87 the only real problem is
util.Uint*types. Coincidentally, it'd be easier for us to use different native types there (avoiding conversion we have currently).