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

CircleCI publishes outdated plot-schema.json #3159

Closed
antoinerg opened this issue Oct 25, 2018 · 2 comments
Closed

CircleCI publishes outdated plot-schema.json #3159

antoinerg opened this issue Oct 25, 2018 · 2 comments
Assignees
Labels
bug something broken

Comments

@antoinerg
Copy link
Contributor

antoinerg commented Oct 25, 2018

On CircleCI publish happens right after the build job.

The build job on the CI is npm run cibuild which doesn't update the plot-schema.json. We should update it either by:

  1. adding code to https://github.com/plotly/plotly.js/blob/master/tasks/cibundle.js
  2. creating a new task only for that purpose which would only be executed in the publish job.

Option 1 adds 15 seconds to the build step. So I would favor option 2 since it can be runned in parallel.

Poor's man benchmark

master

[antoine@newton:~/plotly/plotly.js]$ time npm run cibuild

> plotly.js@1.41.3 cibuild /home/antoine/plotly/plotly.js
> npm run preprocess && node tasks/cibundle.js

###
> plotly.js@1.41.3 preprocess /home/antoine/plotly/plotly.js
> node tasks/preprocess.js

ok plotly-geo-assets.js
ok plotly.js
ok plotly.min.js

real    0m39.960s
user    0m59.195s
sys     0m2.339s

Option 1

[antoine@newton:~/plotly/plotly.js]$ time npm run cibuild

> plotly.js@1.41.3 cibuild /home/antoine/plotly/plotly.js
> npm run preprocess && node tasks/cibundle.js


> plotly.js@1.41.3 preprocess /home/antoine/plotly/plotly.js
> node tasks/preprocess.js

ok plotly-geo-assets.js
ok plotly-with-meta.js
ok plot-schema.json
ok plotly.js
ok plotly.min.js

real    0m53.048s
user    1m21.699s
sys     0m3.376s
@antoinerg antoinerg added bug something broken type: maintenance labels Oct 25, 2018
@antoinerg antoinerg self-assigned this Oct 25, 2018
@antoinerg antoinerg changed the title CircleCI publish outdated plot-schema.json CircleCI publishes outdated plot-schema.json Oct 25, 2018
@etpinard
Copy link
Contributor

Voting for just calling

// Browserify plotly.js with meta and output plot-schema JSON
tasks.push(function(cb) {
_bundle(constants.pathToPlotlyIndex, constants.pathToPlotlyDistWithMeta, {
standalone: 'Plotly',
debug: DEV,
noCompress: true
}, function() {
makeSchema(constants.pathToPlotlyDistWithMeta, constants.pathToSchema)();
cb();
});
});

in the publish step.

@antoinerg
Copy link
Contributor Author

antoinerg commented Oct 25, 2018

Sounds like a very good idea! I will remove the PR #3160 I just made! Actually, PR #3160 does exactly that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

No branches or pull requests

2 participants