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

feat: fetch extra_properties types dynamically from Katsu #266

Merged
merged 10 commits into from
Aug 17, 2023
Merged

feat: fetch extra_properties types dynamically from Katsu #266

merged 10 commits into from
Aug 17, 2023

Conversation

noctillion
Copy link
Contributor

@noctillion noctillion commented Jul 3, 2023 •
edited
Loading

Extra property schema types fetching and integration

This PR introduces the fetching and integration of extra property schema types into the form selection options.
Related to Feature #1683

@noctillion noctillion marked this pull request as ready for review July 19, 2023 18:03
davidlougheed
davidlougheed requested changes Jul 19, 2023
View reviewed changes
Copy link
Member

@davidlougheed davidlougheed left a comment

Choose a reason for hiding this comment

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

overall looking good! minor requests

src/modules/metadata/reducers.js Outdated Show resolved Hide resolved
src/components/manager/projects/ProjectJsonSchemaForm.js Show resolved Hide resolved
src/modules/metadata/reducers.js Show resolved Hide resolved
@davidlougheed davidlougheed changed the title Features/conf-extra-properties-schema feat: fetch extra_properties types dynamically from Katsu Jul 19, 2023
v-rocheleau
v-rocheleau requested changes Jul 19, 2023
View reviewed changes
Copy link
Contributor

@v-rocheleau v-rocheleau left a comment

Choose a reason for hiding this comment

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

Looking good, small request

src/components/manager/projects/ProjectJsonSchemaModal.js Outdated Show resolved Hide resolved
v-rocheleau
v-rocheleau approved these changes Aug 16, 2023
View reviewed changes
Copy link
Contributor

@v-rocheleau v-rocheleau 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 would like an explanation for this comment too

@noctillion
Copy link
Contributor Author

Lgtm, but would like an explanation for this comment too

Sure thing!! Thanks for your review and insights!!

davidlougheed
davidlougheed requested changes Aug 17, 2023
View reviewed changes
Copy link
Member

@davidlougheed davidlougheed left a comment

Choose a reason for hiding this comment

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

very minor tweaks, otherwise looks good!

src/components/manager/projects/ProjectJsonSchemaModal.js Outdated Show resolved Hide resolved
src/modules/metadata/reducers.js Outdated Show resolved Hide resolved
@noctillion
Rename state variable
fbdba34
@noctillion
Add extraProperties
81f7f86 
SchemaTypes selector with loading state
@noctillion
Copy link
Contributor Author

very minor tweaks, otherwise looks good!

Thanks for the insights!! 💯

davidlougheed
davidlougheed approved these changes Aug 17, 2023
View reviewed changes
Copy link
Member

@davidlougheed davidlougheed left a comment

Choose a reason for hiding this comment

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

lgtm

@noctillion noctillion merged commit 626f2fd into master Aug 17, 2023
3 checks passed
@davidlougheed davidlougheed deleted the features/conf-extra-properties-schema branch August 17, 2023 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidlougheed davidlougheed davidlougheed approved these changes

@v-rocheleau v-rocheleau v-rocheleau 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
@noctillion @davidlougheed @v-rocheleau