-
Notifications
You must be signed in to change notification settings - Fork 3
add nix setup for esp32 atomvm development #7
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
Conversation
UncleGrumpy
left a comment
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 looks good, just a couple of details that need fixing and consideration.
tutorials.md
Outdated
| mbedtls_2 | ||
| zlib | ||
| ninja | ||
| doxygen |
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.
doxygen should only be needed if you are building the AtomVM documentation.
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.
i think this is fine, bc this flake provides the development environment to develop AtomVM on ESP32, including the docs
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.
Then you will also need sphinx installed, doxygen alone is useless, you will need the rest of the requirements to build the docs:
- rebar3
- graphviz
- Sphinx and other python deps found in AtomVM/doc/requirements.txt
Check out the docs/README.md for details:
https://github.com/atomvm/AtomVM/blob/main/doc/README.md
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.
rebar3 is installed
beamMinimal27Packages.rebar3
this is the name is given in nix packages since this rebar3 is compiled with Erlang27
you can replace 27 for 28 and it will get the rebar3 compiled with erlang28
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.
I spotted rebar3 right after I posted, was just about to edit that comment. ;-)
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.
We are recommending OTP-28 for builds on main branch, and 27 for release-0.6 since that branch doesn't support OTP-28.
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.
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.
the guide explains you should change the versions for your specific builds, maybe we should add a note about this ??
We are recommending OTP-28 for builds on main branch, and 27 for release-0.6 since that branch doesn't support OTP-28.
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 looks much better. Thank you for the contribution! I left another follow up comment about doxygen and the documentation.
|
We will also want you to squash and signoff the commit before we merge. |
|
Unfortunately this now documentation site is not set up to be built and served locally anymore like our old one, so I cant test what the final rendered tutorial will look like, but otherwise spelling and grammar all looks fine to me. |
|
@UncleGrumpy , what do you mean sign off ? remove the GPG commit signing ?? |
No, keep that… use |
Signed-off-by: Pepe Márquez Romero <pepe.marquezromero@protonmail.com>
Signed-off-by: Pepe Márquez Romero <pepe.marquezromero@protonmail.com>
…hon env from the esp-idf and add graphviz Signed-off-by: Pepe Márquez Romero <pepe.marquezromero@protonmail.com>
Signed-off-by: Pepe Márquez Romero <pepe.marquezromero@protonmail.com>
rebased with the option --signoff commit messages should be fine now |
The Readme was just copied over from the old repo. It would be good to clean it up a little and update how to build locally for testing in a separate PR. I don’t think we necessarily want to merge the Gemfile and config yaml, that might disturb how it’s currently being served. Since they are short the content can be added to the Readme with instructions. |
i can update the instructions in a separate PR done in #8 |
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 looks great. Thank you again for the contribution. @bettio Will have to give this the final approval and do the merge.
If you squash or flatten your commits into the first one that would be great too. We don’t need our git history to include all the revisions during review, just have the meaningful commit messages with the actual changes being merged.
i will squash, it will take the PR name as a commit |
You probably want to change the PR description here to something more informative about the changes, so it can be read from the CLI and know what it's about without having to open a browser to look at the other PR to get a description. ;-) |
|
@bettio , could you review please ? |
|
Thank you for updating the PR description. I did forget to mention we do have another little bit-picky detail that we use throughout the AtomVM organization repos, we like to capitalize the first word of the commit title (no end punctuation). |


guide for setting up a development environment for AtomVM ESP32 in Nixos. It defines the toolchain is needed for compiling from source the AtomVM.