-
Notifications
You must be signed in to change notification settings - Fork 98
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
chore: Readme clarity improvements #1271
chore: Readme clarity improvements #1271
Conversation
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.
lgtm
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.
Thanks a lot. I left a a nit about the wording for Python.
py/bitbox02/README.md
Outdated
such as regenerating files have their libraries NOT in requirements.txt as they are not always necessary to run python | ||
scripts. |
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.
suggestion:
not always necessary to run the python scripts => not necessary for the bitbox02 Python package.
They are never necessary for the bitbox02 package, as these are dev-dependencies not relevant for the package users.
Please rebase, there appears to be a conflict already 😇 |
a5053c6
to
808e6cf
Compare
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.
Thanks. Please squash so there is one commit instead of two.
808e6cf
to
89805cd
Compare
BUILD.md
Outdated
`make dockerdev` expecting specific version of the image. | ||
|
||
```sh | ||
docker pull shiftcrypto/firmware_v2:latest |
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.
So sorry for missing this 😇, but since last week we don't use :latest
but the version in .containerversion
. See https://github.com/BitBoxSwiss/bitbox02-firmware/pull/1282/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R134
Please change this line to make dockerpull
.
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.
Will amend the commit so it stays as 1 in a minute
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.
Should be up to date
Reword the python readme based on suggestion from comments. Signed-off-by: RostarMarek <rostarmarek@gmail.com> chore: Readme clarity improvements Added a few parts to readme files to incentivize using Docker setup instead of local environment. Signed-off-by: RostarMarek <rostarmarek@gmail.com>
89805cd
to
fa6c437
Compare
Added a few parts to readme files to incentivize using Docker setup instead of local environment. Also made sure these aprts link to dockerize setup in Build.md.
Once I have a better understanding of simulator and scripts I will also probably try to write up some at least small guide for those.