scheme-containers / monorepo

Generators, website, issues
https://containers.scheme.org
MIT License
7 stars 1 forks source link

Added lips #22

Closed Retropikzel closed 2 months ago

Retropikzel commented 2 months ago

This only has head and it installs from npm. It's not ideal but the repository did not have simple way to build and install and since the implementation is still in beta I did not use too much time and energy on this. Feel free to deny this pull request if this is a problem.

It does only contain the head because in here https://github.com/LIPS-scheme/lips it says that:

 NOTE: The version that is on NPM is heavily outdated, use beta version

So I dont think theres much point adding an old version.

lassik commented 2 months ago

Thank you for adding another Scheme. Your judgment calls make sense to me.

For most schemes we have a separate build and run containers. With nodejs does it make sense to do the npm install in the run container? I'm not well versed in npm, and not a Docker guru either.

lassik commented 2 months ago

By separate build and run containers, I mean two FROM lines in the same Dockerfile. E.g.

FROM debian:bookworm-slim AS build
...

FROM debian:bookworm-slim
...
Retropikzel commented 2 months ago

Thank you for adding another Scheme. Your judgment calls make sense to me.

For most schemes we have a separate build and run containers. With nodejs does it make sense to do the npm install in the run container? I'm not well versed in npm, and not a Docker guru either.

I'm not sure, I will separate them and see what happens.

Retropikzel commented 2 months ago

Thank you for adding another Scheme. Your judgment calls make sense to me. For most schemes we have a separate build and run containers. With nodejs does it make sense to do the npm install in the run container? I'm not well versed in npm, and not a Docker guru either.

I'm not sure, I will separate them and see what happens.

It needs nodejs on the running side.

lassik commented 2 months ago

Thanks for trying it out.

It needs nodejs on the running side.

Yes. That's necessary since node doesn't compile standalone executables.

The following issues are unclear to me:

Retropikzel commented 2 months ago

Thanks for trying it out.

It needs nodejs on the running side.

Yes. That's necessary since node doesn't compile standalone executables.

The following issues are unclear to me:

* Does it need npm in addition to node?

No.

* What happens if `npm install` is done in the build container, and the run container merely does the usual `COPY --from=build /usr/local /usr/local`?

It works. Current code does exactly this. Could you test it too thought?

lassik commented 2 months ago

Does it need npm in addition to node?

No.

Good to know.

It works. Current code does exactly this. Could you test it too thought?

Great. Can you commit the scheme-banner fix? I'll try building the result, and merge if it works.

Retropikzel commented 2 months ago

Thank you!