Fix the stlink binary path #1726
Conversation
LaszloLango
added
the
jerry-port
akosthekiss
left a comment
akosthekiss
left a comment
There was a problem hiding this comment.
BTW, the reference to stlink build instructions, well, that's not working. That #build-from-sources deep-linking anchor is unnecessary (and should not be used as the readme of stlink might change over time).
targets/nuttx-stm32f4/README.md
Outdated
| git clone https://bitbucket.org/nuttx/apps.git | ||
| git clone https://github.com/texane/stlink.git | ||
| cd stlink | ||
| git checkout 3d24377c2878fc328fb09b67cdfbda529d209417 |
There was a problem hiding this comment.
I'm not sure I agree with this. Why do we have to fix stlink to a given hash? In the commit message you've mentioned that this saved us from the need for frequent updates to the readme. But what is changing so frequently in stlink that triggers updates here?
If we specify the hash here, we will have a "last known good version", for sure. But we lose any potential benefit from the development that happens on stlink master. And if we wanted to follow up on that then we had to update readme with new "last known good version" hashes over and over again.
There was a problem hiding this comment.
@akosthekiss Okey, I agree with that. What do you want, just update the changed path for the binary, and that's all?
There was a problem hiding this comment.
I indeed suggest to limit the changes to section 5. The file path to the stlink binary is one thing. The other is the flashing instructions URL, where I'd suggest to link to the repository only, without the #build-from-sources (which does not exist).
There was a problem hiding this comment.
Updated following your opinion.
polaroi8d
force-pushed
the
stlink
branch
2 times, most recently
from
c6ed550 to
81bdd20
Compare
akosthekiss
left a comment
akosthekiss
left a comment
There was a problem hiding this comment.
Other than the stylistic comment, LGTM
targets/nuttx-stm32f4/README.md
Outdated
| Connect Mini-USB for power supply and connect Micro-USB for `NSH` console. | ||
|
|
||
| To configure `stlink` utility for flashing, follow the instructions [here](https://github.com/texane/stlink#build-from-sources). | ||
| To configure `stlink` utility for flashing, follow the instructions in the official Stlink repository [here](https://github.com/texane/stlink). |
There was a problem hiding this comment.
Consider this nitpicking, but why not put the link on "Stlink repository", why is "here" needed?
JerryScript-DCO-1.0-Signed-off-by: Levente Orban orbanl@inf.u-szeged.hu
3 participants
Fix the Stlink path for the binary file. Add to checkout for a single commit hash, and we don't need to update the README all the time when something is changes.
JerryScript-DCO-1.0-Signed-off-by: Levente Orban orbanl@inf.u-szeged.hu