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.

Sign up for GitHub

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

Jump to bottom

fix: migrate ot openapi-typescript and openapi-fetch #635

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

fix: migrate ot openapi-typescript and openapi-fetch #635

wants to merge 2 commits into from

Conversation

dgolovin
Copy link
Collaborator

Fix #565.

@dgolovin dgolovin force-pushed the i86-openapi-fetch branch from d7f7ce6 to 0a0108f Compare March 11, 2025 22:48
@benoitf
Copy link
Collaborator

benoitf commented Mar 11, 2025

hello, have a question: why the gen files are included in the source code. Usually it's part of the build process where generated files are generated by a openapi-typescript call ?

https://github.com/containers/podman-desktop-extension-ai-lab/blob/bd01b12d498df52189afccba1bec14cd3982f081/packages/backend/package.json#L93

@dgolovin
Copy link
Collaborator Author

I like when I have everything tagged during release, even generated files, so I don't have to generate them when checking out tag or branch. But that is not critical and I can follow the the same pattern as AI Lab.

@dgolovin dgolovin mentioned this pull request Mar 12, 2025
Closed
@dgolovin
Copy link
Collaborator Author

Applying fix for #636 is not enough to fix #565. The error from #565 still persists. We can do release only after this PR is merged.

@dgolovin dgolovin force-pushed the i86-openapi-fetch branch from 6294be2 to 930bb57 Compare March 13, 2025 00:59
benoitf
benoitf reviewed Mar 13, 2025
View reviewed changes
Copy link
Collaborator

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

LGTM but generated files should not be committed (as in other repositories) and there is a missing command to regenerate the files

benoitf
benoitf reviewed Mar 13, 2025
View reviewed changes
benoitf
benoitf reviewed Mar 13, 2025
View reviewed changes
dgolovin
dgolovin commented Mar 13, 2025
View reviewed changes
dgolovin
dgolovin commented Mar 14, 2025
View reviewed changes
@dgolovin dgolovin requested a review from benoitf March 14, 2025 21:13
@dgolovin
Copy link
Collaborator Author

LGTM but generated files should not be committed (as in other repositories) and there is a missing command to regenerate the files

Updated PR and tested manually all combination creating and reusing API resources.

@dgolovin dgolovin force-pushed the i86-openapi-fetch branch from 5c06a8c to 6e4bb18 Compare March 14, 2025 21:17
@dgolovin dgolovin force-pushed the i86-openapi-fetch branch from 6e4bb18 to 130d83b Compare March 14, 2025 21:19
benoitf
benoitf approved these changes Mar 15, 2025
View reviewed changes
Copy link
Collaborator

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

I would have put generated source code in another sec folder ( like the other extensions) but can be follow up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benoitf benoitf benoitf approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@dgolovin @benoitf