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

Storage: Extract returned object hydration into function #88012

Merged
merged 12 commits into from
May 28, 2024

Conversation

leonorfmartins
Copy link
Contributor

@leonorfmartins leonorfmartins commented May 17, 2024

What is this feature?

enrichObject is called multiple times and holds some internal object of which fields should be overridden depending on the case.
This PR extracts as much logic as possible into that functions and adds tests for it.

Why do we need this feature?

[Add a description of the problem the feature is trying to solve.]

Who is this feature for?

[Add information on what kind of user the feature is for.]

Which issue(s) does this PR fix?:

Part of https://github.com/grafana/search-and-storage-team/issues/5

Special notes for your reviewer:

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@grafana-delivery-bot grafana-delivery-bot bot added this to the 11.1.x milestone May 17, 2024
@leonorfmartins leonorfmartins added the no-changelog Skip including change in changelog/release notes label May 17, 2024
@leonorfmartins leonorfmartins marked this pull request as ready for review May 17, 2024 10:51
@leonorfmartins leonorfmartins requested a review from a team as a code owner May 17, 2024 10:51
@suntala suntala changed the title Extract returned object hydration into function Storage: Extract returned object hydration into function May 21, 2024
Copy link
Contributor

@suntala suntala left a comment

Choose a reason for hiding this comment

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

Nice. It needed some refactoring. Mostly have some naming details to discuss but otherwise looks good to go.

pkg/apiserver/rest/dualwriter_mode2.go Outdated Show resolved Hide resolved
pkg/apiserver/rest/dualwriter_mode2.go Outdated Show resolved Hide resolved
pkg/apiserver/rest/dualwriter_mode2.go Outdated Show resolved Hide resolved
pkg/apiserver/rest/dualwriter_mode2.go Show resolved Hide resolved
@leonorfmartins leonorfmartins force-pushed the dualwriter/labels_test branch 2 times, most recently from dd70819 to 6d24f29 Compare May 27, 2024 15:38
@leonorfmartins leonorfmartins requested review from grafanabot and a team as code owners May 28, 2024 12:31
@leonorfmartins leonorfmartins requested review from joshhunt, ashharrison90, Ukochka, wbrowne, andresmgot and xnyo and removed request for a team May 28, 2024 12:31
@leonorfmartins leonorfmartins removed request for grafanabot and a team May 28, 2024 13:07
@leonorfmartins leonorfmartins merged commit 502bd56 into main May 28, 2024
12 checks passed
@leonorfmartins leonorfmartins deleted the dualwriter/labels_test branch May 28, 2024 13:14
miguelmolina95 pushed a commit to miguelmolina95/grafana that referenced this pull request Jun 3, 2024
* Extract returned object hydration into function

* Finish writing tests for utils func

* Lint

* Update pkg/apiserver/rest/dualwriter_mode2.go

Co-authored-by: Arati R. <33031346+suntala@users.noreply.github.com>

* Better var naming

* Remove duplicated logic

* Lint

* Fix test

* Lint

* Make type private

* Fix one more test

* Fix test

---------

Co-authored-by: Arati R. <33031346+suntala@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/backend no-changelog Skip including change in changelog/release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants