Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
New commands layout #75
New commands layout #75
Changes from all commits
59c2189
7579dd1
f3f9a87
9c5c30a
b3733c7
bb5ccaf
d19cdb1
2b42e8b
6479f32
67c8588
1f882db
f01617f
433bf62
35a3116
f94cfdc
8f025f2
b77db3d
c15bcfe
87346d5
0aedf98
0e29834
c267b9b
22dc8ea
85393f6
f1446fa
78f0a05
238763d
207485b
76ef27b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an issue I filled with near-cli. It's bad security practice to use a seed phrase as a CLI arg. So perhaps this should be interactive like a password.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is both interactive and non-interactive, and again, I don’t expect users to type the command out manually most of the time, so the parameters are there only for reproducibility and integration within scripts, which I don’t see a reason to limit to use params
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just
view
? Even knowing that as-read-only meansview
I found it confusing and is less intuitive since we useview
everywhere else.Is also a bit long. I know you're love for being explicit, but having some aliases here would be nice.
As a short cut would be very handy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me just saying
view
might lead some people to think that they're viewing the contract code rather than it being viewing the result of a read-only function.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you execute a view call and try to invoke a host function like creating a transaction, you will get an error that says it was banned in a view call. So my point is that we use
view
in our documentation and baked into the runtime, so using it, especially in this context helps people to learn it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@willemneal If you finish up the whole command, you will see that it does not make any sense to save a few characters:
I don't expect anyone to type it out manually as we have interactive mode that guides users through it, and prints out the final command if you want to re-use it later.
contract view
makes sense only to victims of near-cli-js naming. Anyone else will be confused and guess that this command will show you the contract, not calling a function. Yet, the good part of this new approach is that you will be able to implement your own extension and call it asnear-cli view ...
just fine, if you feel you want to have a shortcut.