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

fix(cli) tie example apps to package version #127

Merged
merged 1 commit into from Dec 16, 2019

Conversation

xavdid
Copy link
Contributor

@xavdid xavdid commented Dec 14, 2019

Previously, all versions of the CLI pulled from the master branch when making example apps. This was good because we could fixes to the example apps would be available right away. Less good because example apps might conflict with released versions of the CLI.

This came to a head when I merged #124 and the build broke. Two main issues:

  • Part of the build is calling zapier init --template typescript and zapier build. Now, this test was running using the master branch's typescript template but published core@9.0.0. Because that PR changed the TS configuration, the definitions in core were incompatible and it threw an error.
  • apparently CI isn't run on PRs of forks. There's good reason for this, but I didn't realize that was the case and merged a bad change.

Pinning examples to releases means slower bug fixes (which are infrequent anyway), but more control (and the ability to make breaking changes in the templates without affecting versions in the wild).

@xavdid xavdid changed the title fix(cli) tie example apps to version fix(cli) tie example apps to package version Dec 14, 2019
Copy link
Member

@eliangcs eliangcs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good! I didn't test as I assume you already did.

@xavdid xavdid merged commit d4c9225 into master Dec 16, 2019
@xavdid xavdid deleted the example-apps-tied-to-version branch December 16, 2019 17:51
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.

None yet

2 participants