Skip to content
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

Pin Vega and related dependencies on Dockerfile #720

Merged
merged 1 commit into from
Sep 24, 2021

Conversation

0x2b3bfa0
Copy link
Member

@0x2b3bfa0 0x2b3bfa0 commented Sep 19, 2021

@0x2b3bfa0 0x2b3bfa0 added bug Something isn't working p0-critical Max priority (ASAP) labels Sep 19, 2021
@0x2b3bfa0 0x2b3bfa0 self-assigned this Sep 19, 2021
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to internal September 19, 2021 12:11 Inactive
@casperdcl
Copy link
Contributor

wouldn't it be better to run some tests with the docker images after building but before pushing?

@0x2b3bfa0
Copy link
Member Author

wouldn't it be better to run some tests with the docker images after building but before pushing?

Doesn't look precisely easy/trivial/maintainable/beautiful, but I definitely see the value of running the test suite on the built containers before pushing them.

@casperdcl
Copy link
Contributor

https://github.com/docker/build-push-action/blob/master/docs/advanced/test-before-push.md

would've prevented #726 too

@0x2b3bfa0
Copy link
Member Author

Definitely! If our package can test itself after being installed (turtles all the way down), it would be nice to have this kind of test.

@0x2b3bfa0
Copy link
Member Author

🔔 @iterative/cml, it would be nice to expedite the approval of this pull request.

@casperdcl
Copy link
Contributor

@0x2b3bfa0
Copy link
Member Author

0x2b3bfa0 commented Sep 24, 2021

Installing unpinned vega-lite package breaks Node 12 support setup-cml#40 was closed so what issue does this PR solve?

I have linked both pull requests to the same issue because they cover exactly the same issue, and I can't link pull requests to cross–repository issues without a closing keyword.

why aren't these deps included in package.json?

It looks like we're on the same page: iterative/setup-cml#47 (comment). As far as I can tell, those dependencies are “optional” and only required to run commands like vl2svg in workflows.

@0x2b3bfa0 0x2b3bfa0 merged commit 3819b34 into master Sep 24, 2021
@0x2b3bfa0 0x2b3bfa0 deleted the pin-vega-dependencies branch September 24, 2021 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p0-critical Max priority (ASAP)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Installing unpinned vega-lite package breaks Node 12 support
2 participants