-
Notifications
You must be signed in to change notification settings - Fork 97
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
[ENH] add templates bids_validator #586
Conversation
not tested with "yum" images |
Codecov ReportAll modified and coverable lines are covered by tests ✅ see 1 file with indirect coverage changes 📢 Thoughts on this report? Let us know! |
pinging @lamoglia who inspired me to finally open this PR (also because once this is part of neurodocker this will simplify the recipe for several BIDS app, including the base_validator image). |
this is a great addition! one suggestion is that the base image could have nodejs and npm installed. could we check if npm exists before installing node? if npm already exists, we can use the existing installation to install the bids validator. |
just to understand, why would you want to check this? to avoid wasting build time or to avoid conflicting node / npm versions? AFAIK the typical base image we use ubuntu, debian, fefora do not have node installed, right? |
It’s possible that a user could provide a different base image, for example one that already has nodejs in it. Or perhaps a user could have a custom RUN instruction that installs nodejs. We wouldn’t want to overwrite or reinstall nodejs and npm in those cases. The runtime is less of a concern imho — I would be more concerned about potentially overwriting an existing nodejs and causing unexpexted behavior.On Nov 10, 2023, at 11:42, Remi Gau ***@***.***> wrote:
this is a great addition! one suggestion is that the base image could have nodejs and npm installed. could we check if npm exists before installing node? if npm already exists, we can use the existing installation to install the bids validator.
just to understand, why would you want to check this?
to avoid wasting build time or to avoid conflicting node / npm versions?
AFAIK the typical base image we use ubuntu, debian, fefora do not have node installed, right?
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because your review was requested.Message ID: ***@***.***>
|
Done |
thanks @Remi-Gau :) merge at will! |
closes #565
could build an image with a dockerfile generated with the following