8000 Replace closer with resource package by rallen090 · Pull Request #2864 · m3db/m3 · GitHub
[go: up one dir, main page]

Skip to content

Replace closer with resource package - #2864

Merged
rallen090 merged 20 commits into
masterfrom
ra/close-replaced-with-resource-package
Nov 9, 2020
Merged

rallen090 merged 20 commits into
masterfrom
ra/close-replaced-with-resource-package

Conversation

@rallen090
Copy link
Copy Markdown
Collaborator

What this PR does / why we need it:

x/close is a subset of x/resource and so we've replaced usage of the former with the latter and removed the former.

Special notes for your reviewer:

Does this PR introduce a user-facing and/or backwards incompatible change?:


Does this PR require updating code package or user-facing documentation?:


// When an update arrives, the manager will deliver the update to all registered
// watchers.
RegisterWatcher(l OptionsWatcher) close.SimpleCloser
RegisterWatcher(l OptionsWatcher) resource.Closer
Copy link
Copy Mar 8000 kdown
Collaborator

Choose a reason for hiding this comment

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

I know we're not doing anything about it right now, but we'll eventually want to either (a) promote resource to non-x/ or (b) create per-package interfaces instead of bleeding x/ types as part of non-x/ APIs.

Comment thread src/dbnode/client/connection_pool.go Resolved
return firstErr
}

// TODO(xichen): move closeAll to m3x/close.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👏

Comment thread src/x/resource/types.go Resolved
@codecov
codecov Bot commented Nov 9, 2020 •
Copy link
Copy Markdown

Codecov Report

Merging #2864 (ce720e8) into master (ce720e8) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2864   +/-   ##
======================================
  Coverage    69.4%   69.4%           
======================================
  Files        1099    1099           
  Lines       99879   99879           
======================================
  Hits        69399   69399           
  Misses      25794   25794           
  Partials     4686    4686           
Flag Coverage Δ
aggregator 75.9% <0.0%> (ø)
cluster 85.1% <0.0%> (ø)
collector 84.3% <0.0%> (ø)
dbnode 79.1% <0.0%> (ø)
m3em 74.4% <0.0%> (ø)
m3ninx 70.3% <0.0%> (ø)
metrics 17.2% <0.0%> (ø)
msg 74.2% <0.0%> (ø)
query 56.9% <0.0%> (ø)
x 80.3% <0.0%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce720e8...b075d34. Read the comment docs.

@wesleyk wesleyk left a comment
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nice! LGTM

@robskillington
Copy link
Copy Markdown
Collaborator

LGTM but I don't think you meant to upgrade .ci submodule did you?

@rallen090
Copy link
Copy Markdown
Collaborator Author

LGTM but I don't think you meant to upgrade .ci submodule did you?

Yeah that was by mistake. Fixed.

@rallen090
rallen090 merged commit c6a256d into master Nov 9, 2020
@rallen090
rallen090 deleted the ra/close-replaced-with-resource-package branch November 9, 2020 22:19
soundvibe pushed a commit that referenced this pull request Nov 11, 2020
* master: (28 commits)
  [dbnode] Add claims for index segments volume index (#2846)
  [dbnode] Remove namespaces from example config and integration tests (#2866)
  [dbnode] Resurrect flaky test skip (#2868)
  [aggregator] Fix checkCampaignStateLoop (#2867)
  [dbnode] implement deletion method in namespace kvadmin service (#2861)
  Replace closer with resource package (#2864)
  Add coding style guide (#2831)
  Add GOVERNANCE.md to describe governance (#2830)
  Add COMPATIBILITY.md to describe version compatibility (#2829)
  Refactor etcd config as discovery section with convenience types (#2843)
  Refactor x/lockfile into dbnode/server (#2862)
  [lint] Disable nlreturn linter (#2865)
  [m3cluster] Expose placement algorithm in placement service (#2858)
  [etcd] Set reasonable cluster connection/sync settings by default (#2860)
  [dbnode] Use bits.LeadingZeros64 to improve encoder performance (#2857)
  Cleanup m3nsch leftovers (#2856)
  Update ci-scripts to correct coverage tracking (#2854)
  [aggregator] Process campaign state without waiting for first campaign check interval (#2855)
  Bump go to 1.14 (#2853)
  [query] Remove single series error from M3
  ...
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.

4 participants

0