Skip to content

Fix assets generation #3638

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

Merged
merged 1 commit into from
Jun 5, 2018
Merged

Fix assets generation #3638

merged 1 commit into from
Jun 5, 2018

Conversation

ivanovit
Copy link
Contributor

@ivanovit ivanovit commented May 29, 2018

What is the current behavior?

When we don't find an asset in the default image definitions(image-definitions.json) we skip width and height parsing. Then the image generation fails because width and height are undefined.

What is the new behavior?

Now we set the correct size even we don't find default asset properties.

Fixes ProgressNS/sidekick-feedback#199.

PR Checklist

@rosen-vladimirov rosen-vladimirov self-requested a review May 29, 2018 14:02
@dtopuzov
Copy link
Contributor

run ci

Copy link
Contributor

@rosen-vladimirov rosen-vladimirov left a comment

Choose a reason for hiding this comment

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

Please retarget this fix to release branch, so we can include it in 4.1.1 release

@ivanovit ivanovit changed the base branch from master to release June 1, 2018 10:05
When we don't find asset in the default image definitions(image-definitions.json) we skip width and height parsing. Then the image generation fails.

Now we set the correct size even we don't find default asset properties.

ProgressNS/sidekick-feedback#199
@ivanovit ivanovit force-pushed the IIIvanov/fix-assets-generation-2 branch from 0894a2f to 91f998c Compare June 1, 2018 10:10
@ivanovit
Copy link
Contributor Author

ivanovit commented Jun 1, 2018

ping @rosen-vladimirov

Copy link
Contributor

@rosen-vladimirov rosen-vladimirov left a comment

Choose a reason for hiding this comment

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

After green build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants