docs: compile guide #1575
docs: compile guide #1575
Conversation
yarn-project/noir-compiler/README.md
Outdated
|
|
||
| ## Installation | ||
|
|
||
| To install the package, just run `yarn add @aztec/noir-compiler`. | ||
|
|
||
| ## Usage | ||
|
|
||
| To run the compiler as a CLI tool, run `yarn aztec_noir_compiler compile <path_to_noir_contract_crate>` | ||
| To run the compiler as a CLI tool, first install it and then run: `yarn aztec-compile compile --help` |
There was a problem hiding this comment.
"..., first install the aztec-cli: npm install -g @aztec/cli and then run..."?
There was a problem hiding this comment.
It can be used from both packages! So if you don't want the full-blown aztec cli and just want to compile, you can install the noir compiler only. I'd expect most users to just go via the aztec cli, but it's nice to have a more lightweight alternative. I've added a clarification for this here in the README.
| ``` | ||
| aztec-cli compile ./src/noir | ||
| ``` |
There was a problem hiding this comment.
Is this the path the the directory containing Nargo.toml, or the path to the src directory containing main.nr? Perhaps we could make that clearer by using a more descriptive path name (and by explicitly stating which directory above this command)?
There was a problem hiding this comment.
Oh, and where (which dir) should this commnd be executed from?
There was a problem hiding this comment.
Clarified it, and added a reference to the directory structure doc
| To generate them, include a `--typescript` option in the compile command with a path to the target folder for the typescript files: | ||
|
|
||
| ``` | ||
| aztec-cli compile --typescript ./src/ts/contracts ./src/noir |
There was a problem hiding this comment.
Maybe make the input path name more descriptive (as per another comment).
We might even want to have required named cli parameters for the output-dir and input-dir? (But not for this PR).
There was a problem hiding this comment.
We might even want to have required named cli parameters for the output-dir and input-dir?
Hmm disagree there. If there is a very specific single thing that is the target of a command, I think it makes more sense as a positional argument rather than a named one. In this case, "I want to compile this". As for output dir, I think it's better that it's optional to make the command easier, as long as we're telling the user where things were written to.
There was a problem hiding this comment.
It's interesting, because my intuition is that the ordering of the args (as they are ordered currently) is backwards. I would expect "the directory I'm compiling" to be the 1st argument, and "the directory I'm outputting code to" to be the 2nd argument. (Similar to the unix mv or cp commands).
It was this confusion (relative to my intuition) that led me to propose naming the arguments (to avoid the confusion).
Alternatively, we could copy solc and have a positional argument for the source directory, and an --output flag for the output directory?
Anyway, this could be a separate issue and PR (if at all). I don't want to hold up this lovely page, given that the current explanation accurately reflects the current code.
There was a problem hiding this comment.
Alternatively, we could copy solc and have a positional argument for the source directory, and an --output flag for the output directory?
That's my proposal here. The source directory is the only thing required, and the main object you are referring to. The output directory is optional, so we can keep it behind an option.
Just to make sure it's clear: in this example ./src/ts/contracts is not a positional argument, but the value of the typescript option. But if it wasn't clear to you, maybe we need to reconsider. Or perhaps something as simple as rewriting as --typescript=./src/ts would help, assuming commander supports that syntax?
There was a problem hiding this comment.
Ohhhh, I misunderstood and assumed --typescript was a flag without any arguments.
|
@iAmMichaelConnor thanks for the thorough review! Just pushed an updated version. |
Fixes #1569
Co-authored-by: Michael Connor <[email protected]>
spalladino
force-pushed
the
docs/compile-guide
branch
from
c5f734a to
46773a1
Compare
Fixes #1569