nspcc-dev / neofs-sdk-go

Go implementation of NeoFS SDK
Apache License 2.0
6 stars 14 forks source link

Client key for read operations #462

Closed smallhive closed 1 year ago

smallhive commented 1 year ago

closes #429

smallhive commented 1 year ago

Many commits, to be able to make changes in each place separately. Before merging, we may squash them for few

smallhive commented 1 year ago
  1. Conflicts.
  2. There is a commit about the tests, does that mean that before that commit the tests did not pass? I would not go that way: I think after every commit build and tests should be OK.
  3. The last commit message does not end with a dot and also it seems more like "Created/Is created" rather than "Creates".
  1. Fixed
  2. I agree. Meanwhile, these tests are marked by a special build tag and don't affect test pipelines. But ok, divided commit and moved to another
  3. What do you mean? I don't use dots in commits ends. And didn't catch an idea about meaning :smiley:
cthulhu-rider commented 1 year ago

I think after every commit build and tests should be OK

this makes sense, but sometimes you just add unit test which shows some bug (so test fails), and then add fix commit (which in particular fixes test) so u can feel the impact of new changes. Otherwise, test should always pass, yeah

cthulhu-rider commented 1 year ago

@smallhive i guess last commit's message looks a bit wierd and out-of-context, we could clearly write Touched methods never return ErrMissingSigner because signer is initialized in the constructor, so nil-check is always passed.

but i don't insist, me personally can realize any form

smallhive commented 1 year ago

@smallhive i guess last commit's message looks a bit wierd and out-of-context, we could clearly write Touched methods never return ErrMissingSigner because signer is initialized in the constructor, so nil-check is always passed.

but i don't insist, me personally can realize any form

I just wanted to left some context in extended commit message

cthulhu-rider commented 1 year ago

I just wanted to left some context in extended commit message

and this is great and must have, but iiuc @carpawell was confused by grammar

carpawell commented 1 year ago

Meanwhile, these tests are marked by a special build tag and don't affect test pipelines.

but sometimes you just add unit test which shows some bug (so test fails), and then add fix commit (which in particular fixes test) so u can feel the impact of new changes.

Well, I would not go that way anyway. As it is described in our git page with rules, "This is important to have an ability to bisect problematic changes if there is a need to."

I don't use dots in commits ends.

Oh, well, I was not attentive enough then. Hm, it is not declared explicitly but we have an example there:

subsystem: short description (up to 80 symbols, better up to 60)

Long description with references, links, test data and whatever else is needed to explain what, why and how is changed.

Signed-off-by: Your Name email@example.com

I would interpret that as a rule that says that any sentence should end with a period (if it is not a header). Anyway, I would say that all the commits we have done until that PR end with dots if they contain a body.

  1. [...] And didn't catch an idea about meaning

@smallhive, I meant that I did not get the idea of the "Creates in the constructor". Who creates? What are that words about?

smallhive commented 1 year ago

@smallhive, I meant that I did not get the idea of the "Creates in the constructor". Who creates? What are that words about?

I got it, I updated the description messages in the commits