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

Remove CloseDB method from Keybase interface #5820

Merged
merged 4 commits into from
Mar 18, 2020

Conversation

jgimeno
Copy link
Contributor

@jgimeno jgimeno commented Mar 17, 2020

There is no issue, was commented in a call with @alessio.

Description


For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@jgimeno jgimeno marked this pull request as ready for review March 17, 2020 19:21
@jgimeno jgimeno self-assigned this Mar 17, 2020
@jgimeno jgimeno added C:Keys Keybase, KMS and HSMs R4R labels Mar 17, 2020
Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

ACK -- I assume we didn't need this? Can you confirm @alessio ?

@alessio
Copy link
Contributor

alessio commented Mar 18, 2020

Correct, we don't need it @alexanderbez. It's a leftover dating back to the time when we hadn't yet implemented the lazy keybase.

Copy link
Contributor

@alessio alessio left a comment

Choose a reason for hiding this comment

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

The change should be mentioned in the API Breaking Changes stanza; looks good otherwise

CHANGELOG.md Outdated Show resolved Hide resolved
@jgimeno jgimeno requested a review from alessio March 18, 2020 12:25
@tac0turtle tac0turtle added the A:automerge Automatically merge PR once all prerequisites pass. label Mar 18, 2020
@codecov
Copy link

codecov bot commented Mar 18, 2020

Codecov Report

Merging #5820 into master will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #5820   +/-   ##
=======================================
  Coverage   32.51%   32.51%           
=======================================
  Files         346      346           
  Lines       39016    39012    -4     
=======================================
- Hits        12685    12684    -1     
+ Misses      25064    25061    -3     
  Partials     1267     1267           

Copy link
Contributor

@alessio alessio left a comment

Choose a reason for hiding this comment

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

Good to go

@mergify mergify bot merged commit a5899ba into master Mar 18, 2020
@mergify mergify bot deleted the jonathan/remove-closedb-interface-keyring branch March 18, 2020 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:automerge Automatically merge PR once all prerequisites pass. C:Keys Keybase, KMS and HSMs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants