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

[fix] #3031: Fix the UI/UX of missing configuration parameters #3321

Merged
merged 7 commits into from Apr 24, 2023
Merged

[fix] #3031: Fix the UI/UX of missing configuration parameters #3321

merged 7 commits into from Apr 24, 2023

Conversation

appetrosyan
Copy link
Contributor

Description of the Change

Improve UI/UX of incorrect configuration specification.

Issue

Closes #3031

Benefits

Better UI

Possible Drawbacks

Slightly worse readability of the iroha_cli crate, particularly the main.rs file.

Usage Examples or Tests [optional]

@github-actions github-actions bot added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label Mar 22, 2023
@codecov
Copy link

codecov bot commented Mar 22, 2023 •
edited

Codecov Report

Merging #3321 (9b338ba) into iroha2-dev (a4d5c9f) will decrease coverage by 6.51%.
The diff coverage is 47.01%.

@@              Coverage Diff               @@
##           iroha2-dev    #3321      +/-   ##
==============================================
- Coverage       62.33%   55.82%   -6.51%     
==============================================
  Files             169      158      -11     
  Lines           31218    29885    -1333     
==============================================
- Hits            19459    16684    -2775     
- Misses          11759    13201    +1442
Impacted Files Coverage Δ
cli/src/lib.rs 0.58% <0.00%> (-68.85%) ⬇️
cli/src/main.rs 0.66% <0.00%> (-0.44%) ⬇️
cli/src/samples.rs 0.00% <0.00%> (-61.85%) ⬇️
cli/src/style.rs 0.00% <0.00%> (ø)
cli/src/torii/mod.rs 0.00% <0.00%> (-27.66%) ⬇️
cli/src/torii/routing.rs 0.00% <0.00%> (-57.11%) ⬇️
cli/src/torii/utils.rs 0.00% <0.00%> (-84.85%) ⬇️
client/src/http_default.rs 0.00% <0.00%> (-38.94%) ⬇️
client_cli/src/main.rs 0.25% <0.00%> (+<0.01%) ⬆️
config/base/src/lib.rs 95.08% <ø> (+58.71%) ⬆️
... and 100 more

... and 51 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

mversic
mversic previously requested changes Mar 23, 2023
View reviewed changes
config/src/lib.rs Outdated Show resolved Hide resolved
config/src/iroha.rs Outdated Show resolved Hide resolved
cli/src/main.rs Outdated Show resolved Hide resolved
cli/src/main.rs Outdated Show resolved Hide resolved
ilchu
ilchu reviewed Mar 23, 2023
View reviewed changes
config/src/path.rs Outdated Show resolved Hide resolved
config/src/path.rs Outdated Show resolved Hide resolved
config/src/path.rs Show resolved Hide resolved
config/src/lib.rs Outdated Show resolved Hide resolved
config/src/client.rs Outdated Show resolved Hide resolved
config/base/src/lib.rs Outdated Show resolved Hide resolved
cli/src/lib.rs Outdated Show resolved Hide resolved
logger/src/lib.rs Outdated Show resolved Hide resolved
cli/src/lib.rs Outdated Show resolved Hide resolved
cli/src/main.rs Outdated Show resolved Hide resolved
@ilchu ilchu self-assigned this Mar 23, 2023
@appetrosyan appetrosyan mentioned this pull request Mar 23, 2023
Closed
@appetrosyan appetrosyan requested a review from ilchu March 24, 2023 06:08
@appetrosyan appetrosyan marked this pull request as draft March 24, 2023 06:09
0x009922
0x009922 reviewed Mar 31, 2023
View reviewed changes
cli/src/main.rs Outdated Show resolved Hide resolved
config/base/src/lib.rs Show resolved Hide resolved
config/src/client.rs Show resolved Hide resolved
@appetrosyan appetrosyan assigned 0x009922 and unassigned mversic Apr 13, 2023
@appetrosyan appetrosyan force-pushed the i2-ui-ux branch 3 times, most recently from ac5eff5 to 4ee7a63 Compare April 19, 2023 07:19
@appetrosyan appetrosyan force-pushed the i2-ui-ux branch 3 times, most recently from cf3c803 to 2eaac44 Compare April 19, 2023 07:49
@appetrosyan appetrosyan marked this pull request as ready for review April 19, 2023 07:50
mversic
mversic reviewed Apr 20, 2023
View reviewed changes
config/base/derive/src/documented.rs
field: stringify!(#ident),
error
}
)?
Copy link
Contributor

Choose a reason for hiding this comment

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

tabs

Copy link
Contributor

Choose a reason for hiding this comment

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

still tabs

mversic
mversic previously approved these changes Apr 20, 2023
View reviewed changes
@mversic mversic requested a review from 0x009922 April 20, 2023 06:08
@appetrosyan
[fix] hyperledger#3031: Fix the UI/UX of missing configuration parame…
1221e3f 
…ters

Signed-off-by: Aleksandr Petrosyan <a-p-petrosyan@yandex.ru>
@appetrosyan
[refactor]: improve tracing spans
d618e57 
Signed-off-by: Aleksandr Petrosyan <a-p-petrosyan@yandex.ru>
@appetrosyan
[refactor]: Improve log readability.
e813d4c 
Signed-off-by: Aleksandr Petrosyan <a-p-petrosyan@yandex.ru>
@appetrosyan
[refactor]: Improve log readability.
6cc9a6f 
Signed-off-by: Aleksandr Petrosyan <a-p-petrosyan@yandex.ru>
@appetrosyan
[feature]: Add spantrace capture to color_eyre
095ea43 
Signed-off-by: Aleksandr Petrosyan <a-p-petrosyan@yandex.ru>
@appetrosyan
[refactor]: Extract style module
89877bf 
Signed-off-by: Aleksandr Petrosyan <a-p-petrosyan@yandex.ru>
@appetrosyan
[fix]: regression when overspecified type for param
9b338ba 
Signed-off-by: Aleksandr Petrosyan <a-p-petrosyan@yandex.ru>
mversic
mversic reviewed Apr 24, 2023
View reviewed changes
config/Cargo.toml
@@ -24,6 +24,8 @@ serde_json = "1.0.91"
json5 = "0.4.1"
thiserror = "1.0.38"
derive_more = "0.99.17"
cfg-if = "1.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
cfg-if = "1.0.0"

QuentinI
QuentinI approved these changes Apr 24, 2023
View reviewed changes
cli/src/lib.rs Show resolved Hide resolved
cli/src/style.rs Show resolved Hide resolved
config/src/iroha.rs Show resolved Hide resolved
crypto/src/lib.rs Show resolved Hide resolved
@appetrosyan appetrosyan merged commit d5fbb2a into hyperledger:iroha2-dev Apr 24, 2023
9 of 11 checks passed
@0x009922 0x009922 mentioned this pull request May 12, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mversic mversic mversic approved these changes

@QuentinI QuentinI QuentinI approved these changes

@s8sato s8sato Awaiting requested review from s8sato s8sato is a code owner

@Arjentix Arjentix Awaiting requested review from Arjentix

@SamHSmith SamHSmith Awaiting requested review from SamHSmith SamHSmith is a code owner

@outoftardis outoftardis Awaiting requested review from outoftardis outoftardis is a code owner

@pesterev pesterev Awaiting requested review from pesterev

@Erigara Erigara Awaiting requested review from Erigara Erigara is a code owner

@ilchu ilchu Awaiting requested review from ilchu

@0x009922 0x009922 Awaiting requested review from 0x009922 0x009922 is a code owner

Assignees

@appetrosyan appetrosyan

@ilchu ilchu

@0x009922 0x009922

Labels
iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@appetrosyan @ilchu @QuentinI @0x009922 @mversic