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

Upgrade embed to 3.15 #4661

Closed
wants to merge 1 commit into from
Closed

Conversation

domoritz
Copy link
Member

@domoritz domoritz commented May 30, 2018

I also rename vega3 to just vega4.

Corresponding PR to add vega3 extension: jupyterlab/jupyter-renderers#139

@domoritz
Copy link
Member Author

This can only pass if someone in the jupyterlab org publishes the new npm package, I think.

An unexpected error occurred: "https://registry.npmjs.org/@jupyterlab%2fvega-extension: Not found".

@domoritz
Copy link
Member Author

domoritz commented Jun 13, 2018

@jasongrout Can you help me with this one? I'm happy to update and rebase when we have figured out how I can rename the package.

@jasongrout
Copy link
Contributor

jasongrout commented Jun 13, 2018

Do you imagine that there will ever be parallel releases of a vega2 and vega3 extension? Are there ever any situations where someone will want to have both vega3 and vega4 installed?

@domoritz
Copy link
Member Author

Possibly, but I would say that the jupyterlab package should always track the latest versions and we can publish vega2, vega3, vega4, vega5... packages in https://github.com/jupyterlab/jupyter-renderers/.

@jasongrout
Copy link
Contributor

jasongrout commented Jun 14, 2018

Sure, that makes sense. Right now I'm spending cycles on the services package rework in #4697, but I can help with this after that. CC also @afshin or @ian-r-rose or @saulshanabrook, who might be able to help more quickly.

@ian-r-rose
Copy link
Member

The rename makes sense to me. What do you think @ellisonbg?

I am a little surprised that the tests would be failing without having published the vega package, especially the js tests. Are they working for you locally?

@domoritz domoritz force-pushed the dom/embed-pi branch 2 times, most recently from 6f44501 to add3ac3 Compare June 14, 2018 21:19
@domoritz domoritz changed the title Upgrade embed to 3.14 Upgrade embed to 3.15 Jun 14, 2018
@domoritz domoritz force-pushed the dom/embed-pi branch 4 times, most recently from e0762cc to 39fabbd Compare June 14, 2018 22:56
@domoritz
Copy link
Member Author

After talking to @ian-r-rose, I decided that it's better not to remove the version from the package so we can allow multiple versions at the same time.

@domoritz domoritz mentioned this pull request Jun 15, 2018
primaryFileType: 'vega3',
fileTypes: ['vega3', 'json'],
defaultFor: ['vega3']
name: 'Vega',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if these should still be 'Vega 4' and 'Vega-Lite 2' , in order to not have a name clash if a user wants to install multiple versions on their distribution.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if we track the latest version, it's cleaner to just show the library. I don't have strong opinions here, though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose that's fair. If you feel this is ready, I can publish the current version.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my side, it's ready to go.

@ian-r-rose
Copy link
Member

I'm not really sure how to get the tests working here. It seems like a mistake to have our CI depend on whether the packages in a given PR have been published, or whether they have changed names. Any ideas @afshin or @jasongrout?

@bollwyvl
Copy link
Contributor

bollwyvl commented Jun 20, 2018 via email

@saulshanabrook
Copy link
Member

@bollwyvl Could you extrapolate what you mean by "the whole thing/thing-extension pattern is not
achievable without direct hacking of the staging/node_modules.
"? Also, what is a verdaccio instance and how does it help?

@jasongrout jasongrout added this to the Beta 3 milestone Jun 27, 2018
@bollwyvl
Copy link
Contributor

bollwyvl commented Jun 27, 2018 via email

@ian-r-rose
Copy link
Member

Closing in favor of #4806, which includes this commit. Thanks for your patience @domoritz!

@saulshanabrook
Copy link
Member

I opened #4809 to track the underlying problem of not being able to get CI to pass on unpublished packages, so we can address that at a later date.

@lock lock bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Aug 8, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Aug 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants