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

Add suggestions for all relevant JS commands #345

Merged
merged 42 commits into from
Jul 9, 2024

Conversation

garikbesson
Copy link
Contributor

@garikbesson garikbesson commented May 23, 2024

  1. The PR contains fixes of suggestions for new syntax of commands when users try to run commands with old (JS) syntax
  2. Integration tests are committed only on purpose to review and test the PR. They should be removed before merging.

@frol frol marked this pull request as draft May 26, 2024 18:28
src/js_command_match/add_credentials.rs Outdated Show resolved Hide resolved
src/js_command_match/add_credentials.rs Outdated Show resolved Hide resolved
src/js_command_match/add_key.rs Outdated Show resolved Hide resolved
@frol
Copy link
Collaborator

frol commented Jun 18, 2024

Please, resolve the conflicts and rewrite integration tests into Rust:

If you are new to integration tests in Rust, Rust book has a great chapter: https://doc.rust-lang.org/book/ch11-03-test-organization.html#integration-tests (tip: don't miss the section called "Integration Tests for Binary Crates")

P.S. If you feel it is too much, let's call for help to rewrite the tests into Rust, just let me know

@garikbesson
Copy link
Contributor Author

@frol Hi, could you review the PR again?

That's key things that have changed since the last review:

  1. I removed end-to-end tests written by me in JavaScript
  2. I implemented unit tests that parse args and check how the interpreted commands look like
  3. I skipped some following methods that are deprecated on JS CLI: clean, dev-deploy, evm-call, evm-view, evm-dev-init, js, repl, set-api-key
  4. I skipped validator-related methods: proposals, stake, validators
  5. All interpreted commands now leverage sign-with-keychain option (instead of leveraging the legacy keychain)

CC: @gagdiez

I re-organized the files following the same structure we had in the
NEAR CLI so it is easier to map them 1:1 and thus evaluate them.

I further updated all commands to use the same nomenclature than
used on NEAR CLI.

Finally, I removed the commands that have been deprecated for a while,
since anyways there is two options:
- Either they are migrating from a tool from were they were already deprecated
- They are new to the tool, and thus what is deprecated or not is not important
@gagdiez gagdiez changed the title WIP: fixing suggested commands for js versions Add suggestions for all relevant JS commands Jul 5, 2024
@gagdiez gagdiez requested a review from frol July 5, 2024 14:22
gagdiez
gagdiez previously approved these changes Jul 5, 2024
Copy link
Contributor

@gagdiez gagdiez left a comment

Choose a reason for hiding this comment

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

I have tested all codes and its variants, and the proposed commands work accordingly

src/js_command_match/transactions/status.rs Outdated Show resolved Hide resolved
src/js_command_match/transactions/status.rs Outdated Show resolved Hide resolved
src/js_command_match/keys/list.rs Outdated Show resolved Hide resolved
src/js_command_match/keys/add.rs Outdated Show resolved Hide resolved
src/js_command_match/contract/call.rs Outdated Show resolved Hide resolved
src/js_command_match/contract/call.rs Outdated Show resolved Hide resolved
@gagdiez gagdiez requested a review from frol July 5, 2024 19:00
src/js_command_match/constants.rs Outdated Show resolved Hide resolved
src/js_command_match/constants.rs Outdated Show resolved Hide resolved
@garikbesson
Copy link
Contributor Author

@race-of-sloths please, include my PR in the Race

@race-of-sloths
Copy link

race-of-sloths commented Jul 9, 2024

@garikbesson Thank you for your contribution! Your pull request is now a part of the Race of Sloths!
New Sloth joined the Race! Welcome!

Shows profile picture for the author of the PR

Current status: executed
Reviewer Score
@frol 13

The average score is 13

@garikbesson check out your results on the Race of Sloths Leaderboard! and in the profile

What is the Race of Sloths

Race of Sloths is a friendly competition where you can participate in challenges and compete with other open-source contributors within your normal workflow

For contributors:

  • Tag @race-of-sloths inside your pull requests
  • Wait for the maintainer to review and score your pull request
  • Check out your position in the Leaderboard
  • Keep weekly and monthly streaks to reach higher positions
  • Boast your contributions with a dynamic picture of your Profile

For maintainers:

  • Score pull requests that participate in the Race of Sloths
  • Engage contributors with fair scoring and fast responses so they keep their streaks
  • Promote the Race to the point where the Race starts promoting you
  • Grow the community of your contributors

Feel free to check our website for additional details!

Bot commands
  • For contributors
    • Include a PR: @race-of-sloths include to enter the Race with your PR
  • For maintainers:
    • Assign points: @race-of-sloths score [1/2/3/5/8/13] to award points based on your assessment.
    • Reject this PR: @race-of-sloths exclude to send this PR back to the drawing board.
    • Exclude repo: @race-of-sloths pause to stop bot activity in this repo until @race-of-sloths unpause command is called

Copy link
Collaborator

@frol frol left a comment

Choose a reason for hiding this comment

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

@garikbesson @gagdiez Enormous effort! Thank you for pulling it out!

@race-of-sloths score 13

@frol frol enabled auto-merge (squash) July 9, 2024 21:39
@race-of-sloths
Copy link

🌟 Score recorded!

@frol, thank you for scoring this pull request in the Race of Sloths!

@frol frol merged commit d5839d0 into near:main Jul 9, 2024
7 checks passed
@frol frol mentioned this pull request Jul 6, 2024
frol added a commit that referenced this pull request Jul 9, 2024
## 🤖 New release
* `near-cli-rs`: 0.11.1 -> 0.12.0

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

##
[0.12.0](v0.11.1...v0.12.0)
- 2024-07-09

### Added
- Cover *all* commands from near-cli JS with the new near-cli-rs
suggestions for full compatibility
([#345](#345))
- Added the ability to select HD Path from the ledger
([#362](#362))
- Added loading indicators for "transaction" group commands and improved
the prompt messages
([#358](#358))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/MarcoIeni/release-plz/).
@race-of-sloths
Copy link

✅ PR is finalized!

Your contribution is much appreciated with a final score of 13!
You have received 150 (130 base + 10 weekly bonus + 10 monthly bonus) Sloth points for this contribution

Congratulations @garikbesson! Your PR was highly scored and you completed another monthly streak! To keep your monthly streak make another pull request next month and get 8+ score for it

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.

4 participants