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

Ts migration for openai apps and customized bundle template #112

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

DevAOC
Copy link
Collaborator

@DevAOC DevAOC commented Nov 4, 2024

Please let me know if you have any questions.

@DevAOC DevAOC requested a review from rdraward November 4, 2024 18:56
Copy link
Collaborator

@rdraward rdraward left a comment

Choose a reason for hiding this comment

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

Initial review. No tophat, but I went through the code. A couple questions about types and such

Copy link
Collaborator

@rdraward rdraward left a comment

Choose a reason for hiding this comment

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

another note - we will get errors in Gadget for the bundle extension's generated folder. I think we want to remove it from the .ignore (I'll also update our docs today)

and related to this - once I fix that issue I get a type issue in the bundle extension itself - line 25 in src/run.ts
CleanShot 2024-11-06 at 11 36 11@2x

@DevAOC DevAOC requested a review from rdraward November 11, 2024 20:12
@DevAOC
Copy link
Collaborator Author

DevAOC commented Nov 11, 2024

another note - we will get errors in Gadget for the bundle extension's generated folder. I think we want to remove it from the .ignore (I'll also update our docs today)

and related to this - once I fix that issue I get a type issue in the bundle extension itself - line 25 in src/run.ts CleanShot 2024-11-06 at 11 36 11@2x

I wasn't able to see the type error but I did fix an issue with the tsconfig

Copy link
Collaborator

@rdraward rdraward left a comment

Choose a reason for hiding this comment

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

another note - we should include the name of the required environment variable in the template instructions BUNDLE_FUNCTION_ID instead of just saying "Add this function id to the environment variables of your development environment."

@DevAOC DevAOC requested a review from rdraward November 21, 2024 21:53
Copy link
Collaborator

@rdraward rdraward left a comment

Choose a reason for hiding this comment

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

Bundle 🎩

  • I see duplicate bundle products being created (I only created "meow bundle" once)
    CleanShot 2024-11-26 at 14 33 25@2x

and when I create a bundle, I also see a logs error
CleanShot 2024-11-26 at 14 33 31@2x

The products are in my admin (see above screenshot) but not in my storefront, so it seems that something funky is going on

@DevAOC
Copy link
Collaborator Author

DevAOC commented Nov 28, 2024

The publicationId issue must be why there are duplicate bundles. Did you see retries in the logs for bundle creation?

@DevAOC
Copy link
Collaborator Author

DevAOC commented Nov 28, 2024

The issue is fixed

Copy link
Collaborator

@rdraward rdraward left a comment

Choose a reason for hiding this comment

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

A couple nits based on console errors in the problem drawer but approving because the tophat works well

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.

2 participants