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

refactor(template): replaces /v1/templates with /v1/templates-list #634

Merged
merged 1 commit into from
Jan 15, 2025
Merged

refactor(template): replaces /v1/templates with /v1/templates-list #634

merged 1 commit into from
Jan 15, 2025

Conversation

stalniy
Copy link
Contributor

@stalniy stalniy commented Jan 14, 2025 •
edited
Loading

refs #477

What was done:

  1. replaces /v1/templates with /v1/templates-list
  2. Fixes types in ApiHttpService
  3. Needed to add deploy field in /v1/templates-list because it's used by ./apps/deploy-web/src/components/sdl/ImageSelect.tsx and apps/deploy-web/src/hooks/useGpuTemplates.ts
  4. Search in templates list was previously done by template name or readme. And since readme is not available in response anymore, I used template's summary.

@stalniy stalniy requested a review from a team as a code owner January 14, 2025 15:55
@stalniy stalniy force-pushed the refactor/templates-list branch 2 times, most recently from fc1de31 to c2c7d59 Compare January 14, 2025 17:33
ygrishajev
ygrishajev reviewed Jan 14, 2025
View reviewed changes
Copy link
Contributor

@ygrishajev ygrishajev left a comment •
edited
Loading

Choose a reason for hiding this comment

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

One thing i missed from the prev pr prob, why is the endpoint named template-list? Shouldn't we continue following the naming approach and name it just templates? If it's taken - bump the version?

@stalniy stalniy force-pushed the refactor/templates-list branch from c2c7d59 to dcc986c Compare January 15, 2025 09:35
@stalniy
Copy link
Contributor Author

stalniy commented Jan 15, 2025 •
edited
Loading

One thing i missed from the prev pr prob, why is the endpoint named template-list? Shouldn't we continue following the naming approach and name it just templates? If it's taken - bump the version?

@ygrishajev We discussed this with @baktun14, you can take a look at his comment here

@stalniy stalniy merged commit f81d56e into main Jan 15, 2025
5 checks passed
@stalniy stalniy deleted the refactor/templates-list branch January 15, 2025 16:13
@stalniy stalniy mentioned this pull request Jan 16, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ygrishajev ygrishajev ygrishajev approved these changes

@baktun14 baktun14 baktun14 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@stalniy @ygrishajev @baktun14