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

chore(amplify-util-uibuilder): remove codegen-ui usage #13234

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

Jshhhh
Copy link
Contributor

@Jshhhh Jshhhh commented Sep 18, 2023

Description of changes

Removes codegen-ui dependency from CLI. This doesn't change current behavior for ui component generation. The codegen-ui dependency is not needed anymore due to component codegen service.

  • copies over necessary helper functions/utils from codegen-ui package

Description of how you validated changes

  1. yarn setup-dev
  2. Ran amplify-dev pull in a test app with datastore, and another app with graphql
  3. Verified that ui components are generated for each app

Checklist

  • PR description included
  • yarn test passes
  • yarn cloud-e2e passes
  • Tests are changed or added

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

Copy link
Member

@sobolk sobolk left a comment

Choose a reason for hiding this comment

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

Please get green PR and E2E runs for this change.

  1. You may need to push some commit to trigger PR or kick off manually via yarn cloud-pr
  2. Use yarn cloud-e2e to get e2e tests.

@Jshhhh Jshhhh force-pushed the remove-codegenui branch 10 times, most recently from 3ecfbe0 to 9ab5ba8 Compare September 26, 2023 20:13
letsbelopez
letsbelopez previously approved these changes Sep 26, 2023
Copy link
Member

@sobolk sobolk left a comment

Choose a reason for hiding this comment

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

Please provide testing evidence as per previous comment.

Please update PR description and elaborate more about changes. A solid paragraph would be appropriate given complexity of the change.

Please fill out "Description of how you validated changes" section of PR description.

@Jshhhh
Copy link
Contributor Author

Jshhhh commented Sep 27, 2023

Description updated.
cloud-e2e tests passed

Copy link
Member

@sobolk sobolk left a comment

Choose a reason for hiding this comment

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

Can you please add packages/amplify-util-builder to https://github.com/aws-amplify/amplify-cli/blob/dev/.github/CODEOWNERS and assign @aws-amplify/amplify-cli-admins AND your team ? Similarly how data team defined ownership ?

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

3 participants