Skip to content

abci: minor cleanups in the socket client#3758

Merged
melekes merged 4 commits intomasterfrom
3513-socket-client
Jul 2, 2019
Merged

abci: minor cleanups in the socket client#3758
melekes merged 4 commits intomasterfrom
3513-socket-client

Conversation

@melekes
Copy link
Contributor

@melekes melekes commented Jun 27, 2019

Follow up from #3512

Specifically:

cli.conn.Close() need not be under the mutex (#3512 (comment))
call the reqRes callback after the resCb so they always happen in the same order (#3512)

Fixes #3513

  • Referenced an issue explaining the need for the change
  • Updated all relevant documentation in docs
  • Updated all code comments where relevant
  • Wrote tests
  • Updated CHANGELOG_PENDING.md

Follow up from #3512

Specifically:

    cli.conn.Close() need not be under the mutex (#3512 (comment))
    call the reqRes callback after the resCb so they always happen in the same order (#3512)

Fixes #3513
@melekes melekes requested review from ebuchman and xla as code owners June 27, 2019 12:21
@tac0turtle tac0turtle added ready-for-review T:code-hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity. labels Jun 27, 2019
Copy link
Contributor

@tac0turtle tac0turtle left a comment

Choose a reason for hiding this comment

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

⚡️

@melekes melekes self-assigned this Jun 27, 2019
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 pending CI fix. Also, does this warrant a (pending) change log entry?

@codecov-io
Copy link

codecov-io commented Jul 2, 2019

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.04%. Comparing base (d041476) to head (91f398c).
⚠️ Report is 3029 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3758      +/-   ##
==========================================
+ Coverage   63.89%   64.04%   +0.15%     
==========================================
  Files         217      217              
  Lines       18136    18136              
==========================================
+ Hits        11588    11616      +28     
+ Misses       5568     5549      -19     
+ Partials      980      971       -9     

see 8 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@melekes
Copy link
Contributor Author

melekes commented Jul 2, 2019

Also, does this warrant a (pending) change log entry?

I'd say no

@melekes melekes merged commit e645442 into master Jul 2, 2019
@ebuchman
Copy link
Contributor

Also, does this warrant a (pending) change log entry?

I'd say no

The re-ordering change here might be significant enough to warrant a changelog entry

@ebuchman ebuchman deleted the 3513-socket-client branch July 12, 2019 04:22
melekes added a commit that referenced this pull request Jul 15, 2019
@melekes melekes mentioned this pull request Jul 15, 2019
5 tasks
ebuchman pushed a commit that referenced this pull request Jul 15, 2019
@ebuchman ebuchman mentioned this pull request Jul 15, 2019
5 tasks
iKapitonau pushed a commit to scrtlabs/tendermint that referenced this pull request Sep 30, 2024
…mint#3728) (tendermint#3758)

Running a `go get` should consistently build this project as per the
README. However, the latest patch release of `btcec/v2` is not a proper
Semantic Versioning patch. It removes an error return value from
`ecdsa.SignCompact` function (btcsuite/btcd@e5d15fd). This is
functionally a no-op, as the underlying function was hardcoded to always
return `nil` as error. However, this is still a breaking change
(compiler/build error) for all consumers of this function, as they
historically expected two return values, but now only get one.

Ref: btcsuite/btcd#2211
Ref: tendermint#3531
Ref: tendermint#3536

---

#### PR checklist

- [ ] Tests written/updated
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments<hr>This is an automatic backport of pull request tendermint#3728 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: Valters Jansons <sigv@users.noreply.github.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T:code-hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

abci: minor cleanups in the socket client

6 participants