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

6 ➡️ 5 Backport to fix #636 #641

Merged
merged 1 commit into from
Sep 23, 2024
Merged

6 ➡️ 5 Backport to fix #636 #641

merged 1 commit into from
Sep 23, 2024

Conversation

Crola1702
Copy link
Contributor

🦟 Bug fix

Fixes #636 for gz-common5

Summary

Backport of #630 and #634

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

@iche033 I'd like your review here as code owner

@Crola1702 Crola1702 changed the title 6 ➡️ 5 Backport fix to #636 6 ➡️ 5 Backport to fix #636 Sep 16, 2024
@Crola1702 Crola1702 changed the base branch from gz-common6 to gz-common5 September 16, 2024 18:06
@iche033
Copy link
Contributor

iche033 commented Sep 16, 2024

I think this PR also included #413 in the backport. Is it intended?

@scpeters
Copy link
Member

I think this PR also included #413 in the backport. Is it intended?

it looks like it backports #630. I think that's not needed to fix the tests

@traversaro traversaro mentioned this pull request Sep 22, 2024
Merged
@iche033 @Crola1702
Fix AssimpLoader collada texture coordinates (#634)
df5f965 
Fixes UNIT_AssimpLoader_TEST with assimp 5.4.3. This should fix homebrew CI.

This PR tweaks the way we checks for texture coordinates from assimp. Empty texture coordinate slots are now allowed as of assimp/assimp#5636. The new logic should be compatible with assimp 5.4.3 and prior versions

Signed-off-by: Ian Chen <[email protected]>
@Crola1702 Crola1702 force-pushed the Crola1702/6_to_5_2024-16_09_2024 branch from d525a96 to df5f965 Compare September 23, 2024 15:35
@Crola1702
Copy link
Contributor Author

it looks like it backports #630. I think that's not needed to fix the tests

Good catch! I already applied the fix that we need. Will wait for CI

scpeters
scpeters approved these changes Sep 23, 2024
View reviewed changes
Copy link
Member

@scpeters scpeters left a comment

Choose a reason for hiding this comment

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

LGTM, I think we can rebase-and-merge

@scpeters scpeters merged commit 4dec38d into gz-common5 Sep 23, 2024
12 checks passed
@scpeters scpeters deleted the Crola1702/6_to_5_2024-16_09_2024 branch September 23, 2024 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scpeters scpeters scpeters approved these changes

@marcoag marcoag Awaiting requested review from marcoag marcoag is a code owner

@azeey azeey Awaiting requested review from azeey

@iche033 iche033 Awaiting requested review from iche033

Assignees
No one assigned
Labels
🏛️ ionic
Projects
Core development
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

🧑‍🌾 AssimpLoader.LoadBox tests failing in gz-common-main homebrew
3 participants
@Crola1702 @iche033 @scpeters