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

Return header directly instead of wrapper struct #497

Merged
merged 1 commit into from
Aug 23, 2023

Conversation

smallhive
Copy link
Contributor

closes #487

client/object_get.go Outdated Show resolved Hide resolved
@@ -349,7 +349,7 @@ type ResObjectHead struct {

// ReadHeader reads header of the requested object.
// Returns false if header is missing in the response (not read).
func (x *ResObjectHead) ReadHeader(dst *object.Object) bool {
func (x *resObjectHead) ReadHeader(dst *object.Object) bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can it be made private now?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resObjectHead is private, so it doesn't matter much.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure but usually when i see public methods on a private struct i think that it implements something and all. that is just an internal converter. if there is no external tests for it, i suggest to make it private. do not insist

client/object_get.go Outdated Show resolved Hide resolved
@smallhive smallhive force-pushed the 487-objecthead-must-return-object-header branch from dd3557d to b82cdc8 Compare August 23, 2023 07:55
@smallhive
Copy link
Contributor Author

Removed resObjectHead completely

@carpawell
Copy link
Member

So to draft or not to draft? That is the question.

@smallhive smallhive marked this pull request as ready for review August 23, 2023 09:13
@smallhive
Copy link
Contributor Author

No draft, of course 😅

@roman-khimov
Copy link
Member

Conflicts after #491 merge.

@smallhive smallhive force-pushed the 487-objecthead-must-return-object-header branch from b82cdc8 to 25d5593 Compare August 23, 2023 10:20
@smallhive
Copy link
Contributor Author

Rebased

@roman-khimov roman-khimov merged commit 2598ff5 into master Aug 23, 2023
5 checks passed
@roman-khimov roman-khimov deleted the 487-objecthead-must-return-object-header branch August 23, 2023 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ObjectHead must return object header
3 participants