Skip to content

IBX-1085: Skipped schema generation for invalid Image Variation #108

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
Merged
merged 8 commits into from
Oct 22, 2021
Merged

IBX-1085: Skipped schema generation for invalid Image Variation #108

merged 8 commits into from
Oct 22, 2021

Conversation

@adamwojs
Copy link
Member

@adamwojs adamwojs commented Oct 18, 2021

JIRA: https://issues.ibexa.co/browse/IBX-1085

Description

As discussed with @bdunogier we should skip schema generation for invalid Image Variation (according to GraphQL spec) and emit warning to logs (and console output):

Skipped schema generation for Image Variation with identifier '50_square'. Please rename given image variation according to GraphQL specification (http://spec.graphql.org/June2018/#sec-Names)

@adamwojs adamwojs changed the title IBX-1085: Skipped schema generation for invalid Image Variation (according to GraphQL spec) IBX-1085: Skipped schema generation for invalid Image Variation Oct 18, 2021
@adamwojs adamwojs marked this pull request as ready for review October 18, 2021 11:23
@adamwojs adamwojs requested review from a team, bdunogier and webhdx October 18, 2021 11:23
alongosz
alongosz requested changes Oct 18, 2021
View reviewed changes
Steveb-p
Steveb-p approved these changes Oct 18, 2021
View reviewed changes
@adamwojs adamwojs requested a review from alongosz October 18, 2021 11:42
Steveb-p
Steveb-p reviewed Oct 18, 2021
View reviewed changes
@ciastektk ciastektk requested a review from a team October 18, 2021 12:10
@adamwojs adamwojs self-assigned this Oct 19, 2021
@adamwojs adamwojs mentioned this pull request Oct 19, 2021
Merged
kacper-wieczorek-ibexa
kacper-wieczorek-ibexa approved these changes Oct 20, 2021
View reviewed changes
Copy link

@kacper-wieczorek-ibexa kacper-wieczorek-ibexa 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.

QA - Approved.

Tested on v3.3.10 and v2.5.24

After patch application - error is absent.

@bdunogier
Copy link
Contributor

bdunogier commented Oct 20, 2021

Good for me.

Note that this should be documented ! Maybe in the image variation doc ?

@DominikaK DominikaK added the Doc needed The changes require some documentation label Oct 20, 2021
@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 22, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@adamwojs adamwojs merged commit 7431563 into 1.0 Oct 22, 2021
@adamwojs adamwojs deleted the ibx_1085 branch October 22, 2021 10:03
@adamwojs
Copy link
Member Author

adamwojs commented Oct 22, 2021

Waiting for merge up for #109 merge

@DominikaK DominikaK removed the Doc needed The changes require some documentation label Oct 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Steveb-p Steveb-p Steveb-p approved these changes

@alongosz alongosz alongosz approved these changes

@ciastektk ciastektk ciastektk approved these changes

@bdunogier bdunogier Awaiting requested review from bdunogier

@webhdx webhdx Awaiting requested review from webhdx

+1 more reviewer

@kacper-wieczorek-ibexa kacper-wieczorek-ibexa kacper-wieczorek-ibexa approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@adamwojs adamwojs

Labels

QA approved

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@adamwojs @bdunogier @Steveb-p @alongosz @ciastektk @kacper-wieczorek-ibexa @DominikaK