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

[cli] Update new cli for latest flipper #70

Closed
wants to merge 8 commits into from

Conversation

shawntabrizi
Copy link
Contributor

@shawntabrizi shawntabrizi commented Apr 22, 2019

  • Fixes the test in the example contract
  • Changes some hard coded instances of "Flipper" to use the desired variable name
  • Calls println when the getter is called

Tested on MacOS

@codecov-io
Copy link

codecov-io commented Apr 22, 2019

Codecov Report

Merging #70 into master will decrease coverage by 0.16%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #70      +/-   ##
==========================================
- Coverage   84.23%   84.06%   -0.17%     
==========================================
  Files          59       59              
  Lines        4547     4556       +9     
==========================================
  Hits         3830     3830              
- Misses        717      726       +9
Impacted Files Coverage Δ
cli/src/cmd/new.rs 0% <0%> (ø) ⬆️

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 acfdf79...62a99d7. Read the comment docs.

cli/Cargo.toml Outdated
@@ -1,7 +1,7 @@
[package]
name = "cargo-contract"
version = "0.1.0"
authors = ["Robin Freyler <robin@parity.io>", "Parity Technologies <admin@parity.io>"]
authors = ["Parity Technologies <admin@parity.io>"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand this change since I thought that this is already in master, isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

¯_(ツ)_/¯

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah so at the "Converation" tab it says that this branch is out of date and you should update it. Please do so. :S

cli/README.md Outdated
@@ -0,0 +1,3 @@
# Cargo plugin for Ink contracts

A small CLI tool for helping setting up and managing WebAssembly smart contracts written with ink!.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here ...

cli/src/cmd/new.rs Outdated Show resolved Hide resolved
cli/src/cmd/new.rs Outdated Show resolved Hide resolved
# cargo clean
# rm Cargo.lock

CARGO_INCREMENTAL=0 cargo build --release --features generate-api-description --target=wasm32-unknown-unknown --verbose
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why have you removed && here?
I think our approach was to add set -e, wasn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied what was in the build.sh script from the examples. Can you suggest an improvement to this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just add set -e on top of the script for now

@shawntabrizi
Copy link
Contributor Author

@ascjones Will try to help get this fixed...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Sub0
  
Awaiting triage
Development

Successfully merging this pull request may close these issues.

None yet

3 participants