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

STRF-8747: split .stencil file into 2 separate config files #662

Conversation

MaxGenash
Copy link
Contributor

What?

  • Moved code dealing with .stencil file into a separate class.
  • Split .stencil file into 2 separate config files: secrets.stencil.json (contains "accessToken", "githubToken") and config.stencil.json (all other fields), so that themes developers can optionally keep general config data (config.stencil.json) in git and add secrets.stencil.json to .gitignore. Also implemented migration from an old config file format on config read to avoid breaking changes.
  • Increased coverage threshold since we have a higher level at the moment.

Tickets / Documentation

Ticket: STRF-8747
Issue: #523

@MaxGenash MaxGenash requested a review from a team October 29, 2020 21:45
@bookernath
Copy link
Contributor

👏 thanks for doing this.

As some existing users of CLI may be expecting the .stencil file to be used, we should probably release this as a new major version, 4.0.

@bookernath
Copy link
Contributor

@MaxGenash can you also add these files to .gitignore in Cornerstone?

https://github.com/bigcommerce/cornerstone/blob/master/.gitignore#L2

This will help make sure theme developers do not accidentally commit them to source control.

@MaxGenash
Copy link
Contributor Author

@bookernath I implemented smooth backward compatibility, so at the moment when users reference the .stencil file in any CLI command they will receive a message about updating the file, it will be automatically replaced and the users can continue working with CLI, no breaking changes yet.
So I believe we can release these changes as v3.1.0 (feature update) and later drop .stencil file support in the v4 release

Screenshot 2020-10-30 at 11 17 59

lib/StencilConfigManager.js Show resolved Hide resolved
lib/stencil-init.js Show resolved Hide resolved
@bookernath
Copy link
Contributor

@MaxGenash great, good thinking and thank you for clarifying!

@junedkazi
Copy link
Contributor

@MaxGenash can you make sure we are not pushing the credentials file when we do a stencil push or when we make a bundle

@MaxGenash
Copy link
Contributor Author

@junedkazi checked, we are not pushing the credentials file when we do a stencil push or when we make a bundle

@MaxGenash
Copy link
Contributor Author

MaxGenash commented Nov 4, 2020

@bookernath @junedkazi before releasing these changes, need to update the documentation on developer.bigcommerce.com to replace mentions of .stencil with the new files and update videos on YouTube where .stncil file is edited (e.g. https://youtu.be/qgaDX7bhmd8 )

@MaxGenash MaxGenash requested a review from junedkazi November 5, 2020 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants