Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Think about ReadFrom/WriteToV2 one more time #532

Open
carpawell opened this issue Nov 15, 2023 · 5 comments
Open

Think about ReadFrom/WriteToV2 one more time #532

carpawell opened this issue Nov 15, 2023 · 5 comments
Labels
discussion Open discussion of some problem I3 Minimal impact S3 Minimally significant U4 Nothing urgent

Comments

@carpawell
Copy link
Member

carpawell commented Nov 15, 2023

Problem

With ReadFromV2 it mostly always required more lines of code to achieve the same thing with ToV2 and FromV2 that allocate structs inside. We can think about some helpers placed near the methods.

Context

Updating SDK in some old code or writing a new code can be annoying with these totally necessary lines (when there is no need to reuse struct b/w reading or writing protocol conversions). This approach was presented in #170 and we had some long discussions, it was not an easy change.

@carpawell carpawell added question Further information is requested discussion Open discussion of some problem labels Nov 15, 2023
@cthulhu-rider
Copy link
Contributor

normally conversion to and from message to SDK entities should be done in exactly one place: https://pkg.go.dev/github.com/nspcc-dev/neofs-sdk-go/client and https://pkg.go.dev/github.com/nspcc-dev/[email protected]/pkg/network/transport respectively. V2-converters accidentally went beyond these two spaces of the system, so you are experiencing a headache

@carpawell
Copy link
Member Author

carpawell commented Nov 24, 2023

It does not matter if I need to use it in a single package or in a dozen packages, if I write tests, or if I am a regular SDK user. I just do it sometimes. The same way I encode cID and oID and maybe some other things that I forget about now. I just need more lines of (boilerplate) code for no reason (usually).

keep an eye on nspcc-dev/neofs-api-go#395

How can this help me?

@cthulhu-rider
Copy link
Contributor

@carpawell, u've not described the exact problem yet in form: i want this but the only way is to cast into v2. If u have the case, report it

if u have some useful helpers, just propose it in PR or create an enhancement issue

@carpawell
Copy link
Member Author

If u have the case, report it

Why do we need cases for exported methods in the "SDK" repo? We already have them (methods) and we are not gonna delete them. Why do I need this? Why this file has to have refs package imported now? Why do i have to explicitly create a var to fill it in go?

in form: i want this but the only way is to cast into v2

The same way I have to encode/decode addresses, IDs, etc. Why? That is not about V2 only.

create an enhancement issue

We are discussing such an issue right now.

@roman-khimov
Copy link
Member

ReadFrom is supposed to be more efficient in some cases (preallocated things, you know it). In practice we have a lot of cases where we do not have anything preallocated. Additional NewFromV2 could be helpful to drop some repetitive code.

@roman-khimov roman-khimov added U4 Nothing urgent S3 Minimally significant I3 Minimal impact and removed question Further information is requested labels Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Open discussion of some problem I3 Minimal impact S3 Minimally significant U4 Nothing urgent
Projects
None yet
Development

No branches or pull requests

3 participants