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

How should we document/describe package.json? #913

Open
samreid opened this issue Mar 13, 2020 · 7 comments
Open

How should we document/describe package.json? #913

samreid opened this issue Mar 13, 2020 · 7 comments

Comments

@samreid
Copy link
Member

samreid commented Mar 13, 2020

From phetsims/utterance-queue#5 (comment) @zepumph said:

One cleanup question I had was questioning if we have a central place where the schema for a sim (or general) package.json is defined. Should we make this? Otherwise there is no place to document readmeCreatedManually but to look at the usage

Would it make sense to do this in simula-rasa's package.json?

@pixelzoom what do you recommend? After commenting, please reassign to @ariel-phet for delegation.

@pixelzoom
Copy link
Contributor

I don't understand. What is "the schema for a sim (or general) package.json" ?

@pixelzoom pixelzoom assigned samreid and zepumph and unassigned pixelzoom Mar 15, 2020
@pixelzoom
Copy link
Contributor

pixelzoom commented Mar 15, 2020

Perhaps "the schema..." means documentation for all of the fields (and their values) that can be present in package.json, and maybe associated some validation. If so...

That would be a lot easier if we had encapsulated package.json. Then we could have described it in the implementation of that encapsulation, verified the contents of package.json, etc. Instead, we have variations of this repeated throughout the code base:

const packageObject = grunt.file.readJSON( `../${repo}/package.json` );

I seem to recall another GitHub issue where this lack of encapsulation was identified, but I can't find it.

I recommended encapsulating package.json, documenting the "schema" there, verifying what's read from package.json, and using that encapsulation wherever package.json is read. And if there's more than 1 package.json "schema", then create more than 1 such encapsulation.

@pixelzoom pixelzoom assigned ariel-phet and unassigned samreid and zepumph Mar 15, 2020
@pixelzoom
Copy link
Contributor

Assigning to @ariel-phet for delegation, as @samreid requested.

@pixelzoom
Copy link
Contributor

pixelzoom commented Mar 15, 2020

And I forgot to answer this question:

Would it make sense to do this in simula-rasa's package.json?

No, I don't think that's where it belongs. It shouldn't be in a package.json. It should be in the thing that reads package.json. And the thing that reads package.json should also be used by chipper, perennial, and anything that needs info from package.json.

@ariel-phet
Copy link
Contributor

Considering that package.json is pretty fundamental to how we do lots of things (to my understanding), this seems like a good dev meeting issue. Marking as such, and high priority to be discussed.

@mattpen
Copy link
Contributor

mattpen commented Mar 19, 2020

3/19 Dev Meeting:

@zepumph suggests we use a similar approach as how we are treating SimVersion.js. We keep the ground truth in perennial, and automatically copy it to chipper/joist/etc where old versions are needed for reproducible builds.

@samreid suggested that we investigated creating an ES6 module that is a package.json loader instead of a UMD.

@zepumph
Copy link
Member

zepumph commented Mar 19, 2020

Also during the meeting @samreid mentioned that we support a schema-type module for our build-local.json, but that is not true from what I see here in chipper's Gruntfile.js:

  // Handle the lack of build.json
  let buildLocal;
  try {
    buildLocal = grunt.file.readJSON( process.env.HOME + '/.phet/build-local.json' );
  }
  catch( e ) {
    buildLocal = {};
  }

Perhaps the next step will be to do the same thing for build-local.

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

5 participants