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] StructOpt added, code reorganized #618

Closed
wants to merge 28 commits into from
Closed

[CLI] StructOpt added, code reorganized #618

wants to merge 28 commits into from

Conversation

damirka
Copy link
Contributor

@damirka damirka commented Feb 4, 2021

Motivation

Motivation for this PR has been described here: #584
Short: this not-so-big refactor builds a foundation for future extensions and improvements of CLI application.

Closes #604
Closes #599
Closes #584
Closes #277
Closes #376 (adds warning when no inputs are found)
Closes #596 (won't do)

Changelog

  • wrapped everything into StructOpt
  • logging and timers are defined in a single place - commands look cleaner and easier to read
  • all API calls are in single place with proper error handling (no unknowns in this one) - see api code
  • all of the traits and structures feature documentation which can be then built with rustdoc
  • leo login <TOKEN> now works (didn't before)
  • all of the scenarios in leo login work: no token, has token, param missing - everything is in place
  • global params for application are now easily defined
  • custom error classes replaced with inlined anyhow!(), more error handlers added
  • improved error messages for verbosity and better understanding of what really happened
  • integration tests added, use cargo test to use
  • added --quiet or -q option to suppress output
  • adds -f flag to leo test to test specific file(s)

Test Plan

Integration tests for CLI are included

@damirka damirka added feature A new feature. help-wanted Extra attention is needed refactor Refactor or cleanup. labels Feb 4, 2021
@damirka damirka self-assigned this Feb 4, 2021
@codecov
Copy link

codecov bot commented Feb 4, 2021

Codecov Report

Merging #618 (56ec74d) into master (770f660) will decrease coverage by 0.73%.
The diff coverage is 78.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #618      +/-   ##
==========================================
- Coverage   75.63%   74.89%   -0.74%     
==========================================
  Files         518      548      +30     
  Lines       16135    17245    +1110     
==========================================
+ Hits        12204    12916     +712     
- Misses       3931     4329     +398     
Impacted Files Coverage Δ
leo/lib.rs 100.00% <ø> (ø)
leo/main.rs 100.00% <ø> (ø)
leo/commands/package/login.rs 52.94% <52.94%> (ø)
leo/api.rs 62.16% <62.16%> (ø)
leo/commands/update.rs 77.77% <75.00%> (ø)
leo/commands/test.rs 74.07% <75.60%> (ø)
leo/commands/build.rs 75.67% <80.00%> (ø)
leo/commands/setup.rs 52.94% <84.61%> (ø)
leo/context.rs 87.50% <87.50%> (ø)
leo/commands/prove.rs 100.00% <100.00%> (ø)
... and 88 more

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 770f660...5a031d1. Read the comment docs.

@damirka damirka marked this pull request as ready for review February 4, 2021 21:02
@damirka damirka changed the title [WIP][CLI] StructOpt added, code reorganized [CLI] StructOpt added, code reorganized Feb 4, 2021
Copy link
Member

@howardwu howardwu left a comment

Choose a reason for hiding this comment

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

I'd like add, remove, login, logout, and publish without package keyword.

leo/cmd/mod.rs Outdated Show resolved Hide resolved
.rustfmt.toml Outdated Show resolved Hide resolved
@damirka
Copy link
Contributor Author

damirka commented Feb 5, 2021

@howardwu updated PR. Ready for review again.

@damirka
Copy link
Contributor Author

damirka commented Feb 8, 2021

Also fixes #376 - added note in PR description above.

When testing library without inputs user receives a warning that no inputs were found:

  Test Unable to find inputs, ignore this message or put them into /inputs folder
  Test Running 1 tests
  ...

- adds -f flag for leo test command, now you choose files to run
- adds tests for leo test
- improves output of test command
- PathBuf used instead of String in StructOpt
- added dummy-test for logout before login
- shortens test cmd code
@damirka
Copy link
Contributor Author

damirka commented Feb 8, 2021

Reopened as #632 for CI support.

@damirka damirka closed this Feb 8, 2021
@damirka damirka deleted the leo-ref branch February 8, 2021 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature. help-wanted Extra attention is needed refactor Refactor or cleanup.
Projects
None yet
2 participants