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

Release Amplify Codegen at major version 4 #592

Merged
merged 1 commit into from
May 11, 2023
Merged

Release Amplify Codegen at major version 4 #592

merged 1 commit into from
May 11, 2023

Conversation

AaronZyLee
Copy link
Contributor

This reverts commit c1f9f36.

  • fix: use hard code codegen version in schema.js

  • fix lint

Description of changes

Issue #, if available

Description of how you validated changes

Checklist

  • PR description included
  • yarn test passes
  • Tests are changed or added
  • Relevant documentation is changed or added (and PR referenced)
  • Breaking changes to existing customers are released behind a feature flag or major version update
  • Changes are tested using sample applications for all relevant platforms (iOS/android/flutter/Javascript) that use the feature added/modified

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

* Revert "fix: codegen downgrade to version 3 (#589)"

This reverts commit c1f9f36.

* fix: use hard code codegen version in schema.js

* fix lint
@AaronZyLee AaronZyLee requested a review from a team as a code owner May 10, 2023 21:22
Comment on lines +163 to +165
// This is hard-coded for the schema version purpose instead of codegen version
// To avoid the failure of validation method checkCodegenSchema in JS Datastore
// The hard code is starting from amplify codegen major version 4
Copy link
Member

Choose a reason for hiding this comment

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

It might be worth explaining why/when this should be revved. I.e.,

This value should be revved using semver rules when any protocol details expressed in the /models files change.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe more to the point, this comment sounds like, "we're setting this to X because DataStore breaks otherwise." But, the value actually just identifies a protocol version, and that protocol version now deviates from package version as of package version 4.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes this will be the protocol version instead. The current approach is a temporary workaround. I will mark this as a TODO to update the comments once the protocol between codegen and library is well defined after a detailed design

@AaronZyLee AaronZyLee merged commit fefa780 into release May 11, 2023
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.

3 participants