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

Remove WIF and NEP2 support in --wallet argument #1789

Merged
merged 1 commit into from
Sep 19, 2022

Conversation

acid-ant
Copy link
Contributor

@acid-ant acid-ant commented Sep 16, 2022

Closes: #1128

@codecov
Copy link

codecov bot commented Sep 16, 2022

Codecov Report

Merging #1789 (4198ef2) into master (3df98ce) will decrease coverage by 0.06%.
The diff coverage is 100.00%.

❗ Current head 4198ef2 differs from pull request most recent head 918a12f. Consider uploading reports for the commit 918a12f to get more accurate results

@@            Coverage Diff             @@
##           master    #1789      +/-   ##
==========================================
- Coverage   33.48%   33.41%   -0.07%     
==========================================
  Files         353      352       -1     
  Lines       23982    23962      -20     
==========================================
- Hits         8030     8008      -22     
- Misses      15286    15287       +1     
- Partials      666      667       +1     
Impacted Files Coverage Δ
cmd/neofs-cli/internal/key/wallet.go 87.50% <ø> (-6.25%) ⬇️
cmd/neofs-cli/internal/key/raw.go 62.96% <100.00%> (-10.02%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@carpawell carpawell left a comment

Choose a reason for hiding this comment

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

  1. Could you, please, add a commit sign-off? We have a CI step that has failed in that PR.
  2. We also should fix the flag description. E.g. neofs-cli container create -h: -w, --wallet string WIF (NEP-2) string or path to the wallet or binary key
  3. It is common practice to write commit msg imperatively: Removed -> Remove.

cmd/neofs-cli/internal/key/raw.go Outdated Show resolved Hide resolved
cmd/neofs-cli/internal/key/wallet.go Outdated Show resolved Hide resolved
cmd/neofs-cli/internal/key/wallet.go Outdated Show resolved Hide resolved
cmd/neofs-cli/internal/key/raw.go Show resolved Hide resolved
cmd/neofs-cli/internal/key/raw.go Outdated Show resolved Hide resolved
@realloc
Copy link

realloc commented Sep 16, 2022

Please avoid merge commits at rebase at all cost.

fyrchik
fyrchik previously approved these changes Sep 19, 2022
carpawell
carpawell previously approved these changes Sep 19, 2022
@carpawell
Copy link
Member

carpawell commented Sep 19, 2022

@acid-ant, for future: we also try to keep CHANGELOG.md in an actual state after every PR.

@realloc
Copy link

realloc commented Sep 19, 2022

Is there a related issue? If not, could you please take care of labels and projects?

@acid-ant
Copy link
Contributor Author

Is there a related issue? If not, could you please take care of labels and projects?

This is a related issue #1128

@acid-ant
Copy link
Contributor Author

@acid-ant, for future: we also try to keep CHANGELOG.md in an actual state after every PR.

Updated. Please review.

fyrchik
fyrchik previously approved these changes Sep 19, 2022
CHANGELOG.md Outdated Show resolved Hide resolved
Signed-off-by: Anton Nikiforov <an.nikiforov@yadro.com>
@fyrchik fyrchik merged commit bb02913 into nspcc-dev:master Sep 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove WIF support in --wallet argument
4 participants