Skip to content
This repository has been archived by the owner on Jun 3, 2020. It is now read-only.

Upgrade to Abscissa 0.1.0-pre.1 #259

Merged
merged 1 commit into from
Jun 9, 2019
Merged

Upgrade to Abscissa 0.1.0-pre.1 #259

merged 1 commit into from
Jun 9, 2019

Conversation

tony-iqlusion
Copy link
Contributor

I'm preparing another release of Abscissa, which provides the application boilerplate / framework used by Tendermint KMS:

https://github.com/iqlusioninc/abscissa

This commit contains what is hopefully the minimum set of changes required to upgrade to the new version.

Notably it removes all TODOs about boilerplate code that could potentially be replaced by custom derive, as abscissa_derive now provides that functionality.

It also incorporates global application state, and the application as the owner of the configuration.

@tony-iqlusion
Copy link
Contributor Author

Note that there's some potential follow-up work to integrate some new Abscissa features:

  • Move chain::registry::REGISTRY into KmsApplication and access it via app_read() and app_write()
  • Use Abscissa's new thread management and signal-handling functionality

I have omitted these changes for now to keep the upgrade as minimally invasive as possible, and would suggest following up on these after another KMS release.

@tony-iqlusion
Copy link
Contributor Author

Looks like the build failed because Abscissa requires the latest Rust (1.35.0).

@thanethomson @liamsi would it be possible to get a new Docker image published with an updated Rust? Thanks!

Copy link
Contributor

@liamsi liamsi left a comment

Choose a reason for hiding this comment

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

LGTM 👍

atomicwrites = "0.2"
byteorder = "1.2"
bytes = "0.4"
chrono = "0.4"
failure = "0.1"
failure_derive = "0.1"
gumdrop = "0.5"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is only used indirectly (via [derive(Options)], right? (I could not find any sign of #[macro_use] extern crate gumdrop;)

Copy link
Contributor

Choose a reason for hiding this comment

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

I've largely switched to using Rust 2018 conventions for importing macros, where instead of #[macro_use] spooky-action-at-a-distance, macros can be imported by name just like anything else.

This allows a crate like gumdrop to re-export the proc macros from gumdrop_derive so that consumers don't need both. After the 2018 edition was released, pretty much everything with a proc macro crate switched to this convention (including abscissa).

@liamsi
Copy link
Contributor

liamsi commented Jun 5, 2019

would it be possible to get a new Docker image published with an updated Rust? Thanks!

Let me double-check with @thanethomson

@thanethomson
Copy link
Contributor

Looks like the build failed because Abscissa requires the latest Rust (1.35.0).

@thanethomson @liamsi would it be possible to get a new Docker image published with an updated Rust? Thanks!

Done 😁 See #264 and #263 too.

I'm preparing another release of Abscissa, which provides the
application boilerplate / framework used by Tendermint KMS:

https://github.com/iqlusioninc/abscissa

This commit contains what is hopefully the minimum set of changes
required to upgrade to the new version.

Notably it removes all TODOs about boilerplate code that could
potentially be replaced by custom derive, as `abscissa_derive` now
provides that functionality.

It also incorporates global application state, and the application as
the owner of the configuration.
@tarcieri tarcieri force-pushed the abscissa/v0.1.0-pre.1 branch from 4c48980 to 3b3bd7b Compare June 9, 2019 01:37
@tarcieri tarcieri merged commit 20172d9 into master Jun 9, 2019
@tarcieri tarcieri deleted the abscissa/v0.1.0-pre.1 branch June 9, 2019 01:46
@tarcieri tarcieri mentioned this pull request Jun 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants