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.

Sign up for GitHub

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

Jump to bottom

Prep release v0.15.0 #454

Merged
merged 18 commits into from
Jul 20, 2020
Merged

Prep release v0.15.0 #454

merged 18 commits into from
Jul 20, 2020

Conversation

liamsi
Copy link
Member

@liamsi liamsi commented Jul 17, 2020 •
edited
Loading

Sorry if this PR grew too big. I can break it into smaller ones if required but I think it should be straightforward to review.
touches parts of: #388
closes #452

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

liamsi
liamsi commented Jul 17, 2020
View reviewed changes
light-client/Cargo.toml

anomaly = { version = "0.2.0", features = ["serializer"] }
contracts = "0.4.0"
crossbeam-channel = "0.4.2"
derive_more = "0.99.5"
futures = "0.3.4"
prost-amino = "0.5.0"
prost-amino = "0.6.0"
Copy link
Member Author

@liamsi liamsi Jul 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@tarcieri and I recently updated this. This is only to speed up build times. Compare: #368

@liamsi liamsi requested review from romac, brapse and ebuchman July 17, 2020 11:25
liamsi
liamsi commented Jul 17, 2020
View reviewed changes
light-client/Cargo.toml
@@ -1,20 +1,20 @@
[package]
name = "tendermint-light-client"
version = "0.14.0"
version = "0.15.0"
authors = ["Romain Ruetschi <romain@informal.systems>"]
Copy link
Member Author

Choose a reason for hiding this comment

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

@romac who would you like to add here? @brapse for sure, and who else?

Copy link
Member

Choose a reason for hiding this comment

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

$ git shortlog -n -s -- light-client

shows

    22  Romain Ruetschi
     6  Alexander Simmerl
     4  Ismail Khoffi
     1  Ethan Buchman
     1  Shivani Joshi
     1  yihuang

liamsi
liamsi commented Jul 17, 2020
View reviewed changes
light-client/src/fork_detector.rs
/// - If verification fails for any other reason, the
/// witness is deemed faulty.
/// - If verification fails because of lack of trust, we have a potential fork.
/// - If verification fails for any other reason, the witness is deemed faulty.
Copy link
Member Author

Choose a reason for hiding this comment

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

We actually never enforce the rules we have via CI by running fmt via nightly at least there.
See:

tendermint-rs/.rustfmt.toml

Lines 1 to 12 in a556d77

unstable_features = true
# comments
comment_width = 100
wrap_comments = true
# imports
reorder_imports = true
# strings
format_strings = false
max_width = 100

Copy link
Contributor

Choose a reason for hiding this comment

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

we should probably change this.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah let's remove the nightly-only formatting directives.

Copy link
Member Author

Choose a reason for hiding this comment

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

ref: #153 and #121

Either remove them or reject PRs via CI that do not comply with these rules.

@liamsi
Fix rustdoc:
30247c5 
 Documenting tendermint v0.15.0 (/Users/ismail/IdeaProjects/tendermint-rs/tendermint)
error: `[Error::InvalidTrustThreshold]` cannot be resolved, ignoring it.
   --> tendermint/src/lite/types.rs:110:38
    |
110 |     /// In any other case we return [`Error::InvalidTrustThreshold`].
    |                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot be resolved, ignoring
    |
@liamsi
add comment in light-node lib
6065021
@liamsi liamsi marked this pull request as ready for review July 18, 2020 00:28
melekes
melekes previously approved these changes Jul 18, 2020
View reviewed changes
Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

lgtm

@yihuang
Copy link
Contributor

yihuang commented Jul 18, 2020

How about upgrade sled to most recent version 0.33?

@liamsi
Copy link
Member Author

liamsi commented Jul 18, 2020

How about upgrade sled to most recent version 0.33?

It was on my radar (#452) but I didn't try it yet as there might be breaking changes in the sled API. Can try on Monday. Feel free to submit a PR that updates it independently of this one in the meantime.

@liamsi
Copy link
Member Author

liamsi commented Jul 18, 2020

@yihuang seems fine actually: 62822b4

@yihuang yihuang mentioned this pull request Jul 18, 2020
Merged
xla
xla previously requested changes Jul 20, 2020
View reviewed changes
Copy link
Contributor

@xla xla left a comment

Choose a reason for hiding this comment

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

Dope! Just a bunch of typos.

CHANGES.md Outdated Show resolved Hide resolved
light-client/src/light_client.rs Outdated Show resolved Hide resolved
light-client/src/light_client.rs Outdated Show resolved Hide resolved
light-client/src/light_client.rs Outdated Show resolved Hide resolved
light-node/src/lib.rs Outdated Show resolved Hide resolved
light-node/src/rpc.rs Outdated Show resolved Hide resolved
@liamsi @xla
Apply suggestions from code review
3a14d8b 
Co-authored-by: Alexander Simmerl <a.simmerl@gmail.com>
@liamsi liamsi requested a review from xla July 20, 2020 06:56
@liamsi liamsi dismissed xla’s stale review July 20, 2020 06:56

all addressed

xla
xla approved these changes Jul 20, 2020
View reviewed changes
Copy link
Contributor

@xla xla left a comment

Choose a reason for hiding this comment

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

🌋 ☀️ 📣 🔛

liamsi
liamsi commented Jul 20, 2020
View reviewed changes
CHANGES.md
@@ -1,4 +1,9 @@
## Pending
## [0.15.0] (2020-07-17)
Copy link
Member Author

Choose a reason for hiding this comment

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

We should maybe update the date here. I'll merge this for now and publish the crates this afternoon.

@liamsi liamsi merged commit 9dfb7b3 into master Jul 20, 2020
@liamsi liamsi deleted the ismail/release/v0.15.0 branch July 20, 2020 08:47
@liamsi liamsi added this to the v0.15.0 milestone Jul 20, 2020
@liamsi liamsi mentioned this pull request Jul 21, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xla xla xla approved these changes

@melekes melekes melekes left review comments

@romac romac Awaiting requested review from romac

@brapse brapse Awaiting requested review from brapse

@ebuchman ebuchman Awaiting requested review from ebuchman

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.15.0
Development

Successfully merging this pull request may close these issues.

light-client: update sled dependency to latest version
6 participants
@liamsi @yihuang @brapse @xla @romac @melekes