search

studiopress / genesis-sample

This is the sample theme created for the Genesis Framework.
https://demo.studiopress.com/genesis-sample/
528 stars 284 forks source link

Parse package.json for PI Service JSON #333

Closed BMO-tech closed 4 years ago

BMO-tech commented 4 years ago

Summary of change:

Part of the CI/CD process is to supply a consumable zip and .json to the Product Info Service. This PR extracts the theme based values inside of package.json into a consumable .json file for the PI Service.

Have the changes in this PR been added to the documentation for this project?

Does not apply

How to test:

N/A

N/A

N/A

Suggested Changelog Entry:

This is an internal WP Engine update

nickcernis commented 4 years ago

Hi, @bmo-tv! I think it would be fine to read the package.json file directly if you wanted to simplify things and skip style.css parsing.

We read the package.json name and theme version already in the zip script here: https://github.com/studiopress/genesis-sample/blob/1ac588c8b8625c82aabbfc9a4e9d77f472751644/.scripts/makezip.js#L40-L44

And the issue release checklist includes a reminder to bump the version in package.json too.

If there are headers missing from package.json but present in style.css that the product info service needs, we could potentially add those to package.json.

BMO-tech commented 4 years ago

Completely updated the makePIServiceData.js script based on Nicks recommendation to use the values present in package.json. I also removed the .circleci/config.yml changes as they are not relevant to this PR or the scope of the sub-task.

I've noted the comments left for the config.yml however, and will address them in the next PR.

BMO-tech commented 4 years ago

If there is a preference to not rely too heavily on process.env values then I can easily update the script. I chose to use them instead of importing package.json directly because the values are already available at run time. It seems trivial for such a small script, but generally I try not to duplicate available values.

The PI Service will need a .json file outside of the .zip artifact in order to function. We could copy package.json before creating the .zip, but I think it's just as easy to create a new .json file that only contains relevant data.

Good catch on the unique .json name. While every theme will be in it's own unique S3 path (/themes/genesis-sample/), if we follow the Atomic Blocks Pro process, then the .json should contain the version as well: IE: /themes/genesis-sample/genesis-sample.3.2.0.json

nickcernis commented 4 years ago

I chose to use them instead of importing package.json directly because the values are already available at run time. It seems trivial for such a small script, but generally I try not to duplicate available values.

That seems very reasonable. My concern was more about legibility/maintainability, but your version's well commented. I don't have a strong preference for either approach.

The PI Service will need a .json file outside of the .zip artifact in order to function. We could copy package.json before creating the .zip, but I think it's just as easy to create a new .json file that only contains relevant data.

Thanks, I appreciate the info and context. The package.json would include a bunch of data PI doesn't need too. Stripping it down to the theme key makes sense.