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

Dedicated wrappers should be included in build.json on a sim by sim basis #681

Closed
zepumph opened this issue Jun 12, 2018 · 7 comments
Closed
Assignees

Comments

@zepumph
Copy link
Member

zepumph commented Jun 12, 2018

Right now, if we want to add a dedicated wrapper to the build, like the Hooke's Law Energy wrapper in
https://github.com/phetsims/phet-io-wrapper-hookes-law-energy/issues/4, we have to add it in two places in chipper: copySupplementalPhetioFiles, and build.json. I see 2 possible improvements here:

  1. copySupplemental could get its dedicated wrapper repo list from build.json.

  2. buildJSON["phet-io"].phetLibs could look in the sim's package and add any additional phet-io libs that way (I think this is supported for phet, but not phet-io).

This issue (especially number 1), is related to #680

Tagging @samreid and @jonathanolson so they are aware of the problem.

@zepumph zepumph self-assigned this Jun 12, 2018
@samreid
Copy link
Member

samreid commented Jun 12, 2018

Another nice feature would be to add it to the wrappers/index table, something like "Simulation Specific Wrappers Table".

@jonathanolson
Copy link
Contributor

So it's desired for each sim to potentially have some number of built/included wrappers that get included just with that sim?

@zepumph
Copy link
Member Author

zepumph commented Jun 12, 2018

Yes. Currently all of the dedicated wrapper repo wrappers (lab book, classroom activity, and hookes law energy) are only supported by individual sims (CLB & BLL, FAMB, and HL respectively).

The build process is just adding all of these in on every build. It would be nice if we could say "only include lab book for CLB and BLL."

@jonathanolson
Copy link
Contributor

So it sounds like each wrapper has an effective list of sims that it should be included with? Some should only be with one simulation. Some should be a few simulations. Most will be included for all simulations.

Perhaps each wrapper should declare this in its package.json, and the build process will just include all detected wrappers valid for that sim?

@zepumph
Copy link
Member Author

zepumph commented Jun 14, 2018

Wrappers in the wrapper suite are, by design, included in all sim deploys, and so I think this is only a problem in dedicated wrapper repos, and no for items in the wrapper suite.

Currently these repos don't have a package.json, which is why I proposed adding them into the sims' package.json themselves, but I also recognize that it could be nice to have that data in a single place.

@jonathanolson
Copy link
Contributor

Currently these repos don't have a package.json, which is why I proposed adding them into the sims' package.json themselves, but I also recognize that it could be nice to have that data in a single place.

That's fine too, no major preference.

@zepumph
Copy link
Member Author

zepumph commented Sep 27, 2019

This was handled in #785. Closing

@zepumph zepumph closed this as completed Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants