chore: lock in nph version and add pre-commit hook #2938
Conversation
|
You can find the image built from this PR at Built from b1b7420 |
ok, I should be able to do that. Will update. |
Makefile
Outdated
| check_nph_installed: | ||
| ifeq (, $(shell which nph 2> /dev/null)) |
There was a problem hiding this comment.
This is nice indeed! Nevertheless, I think it is better to have this check implicit when the developer performs any make command. I mean, this is a pre-condition that should always be met and I think it will be more secure to enforce that in a generic manner.
There was a problem hiding this comment.
I would disagree with that. If I am a user that just want to build wakunode2 because I want latest master or testing a branch, the presence of nph should not block me to proceed.
README.md
Outdated
| You can install a convenient pre-commit git hook with the following command: | ||
|
|
||
| ``` | ||
| make install_nph_prehook | ||
| ``` |
There was a problem hiding this comment.
Considering that the hook will always be triggered, we don't need the developer to explicitly do make install_nph_prehook. In case my other comment makes sense of course :)
| You can install a convenient pre-commit git hook with the following command: | |
| ``` | |
| make install_nph_prehook | |
| ``` |
There was a problem hiding this comment.
I don't think we should install pre-hooks nilly-willy, especially if a dev has already their own prehooks in place. Which is why I set it up as separate step.
This comment was marked as resolved.
This comment was marked as resolved.
fryorcraken
force-pushed
the
nph-githook
branch
from
d3c5f18
to
c4ad125
Compare
fryorcraken
changed the title
fryorcraken
force-pushed
the
nph-githook
branch
from
c4ad125
to
5458850
Compare
|
|
||
| Nim files are expected to be formatted using the [`nph`](https://github.com/arnetheduck/nph) version present in `vendor/nph`. | ||
|
|
||
| You can easily format file with the `make nph/<relative path to nim> file` command. |
There was a problem hiding this comment.
Shall we have an example of how to format a particular file? That would help to understand the command :)
( maybe it sounds more natural having make nph <relative-path-to-nim-file> )
fryorcraken
force-pushed
the
nph-githook
branch
4 times, most recently
from
4fe7f48
to
73170f1
Compare
fryorcraken
force-pushed
the
nph-githook
branch
from
73170f1
to
121d820
Compare
Description
nphversion tov0.5.1by bringing it as a submodulenph