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

[ign-common3] Support fuel URLs for textures #102

Merged
merged 8 commits into from
Oct 16, 2020
Merged

[ign-common3] Support fuel URLs for textures #102

merged 8 commits into from
Oct 16, 2020

Conversation

JShep1
Copy link

@JShep1 JShep1 commented Oct 7, 2020 •
edited
Loading

Resolves gazebosim/gz-sim#343

Signed-off-by: John Shepherd john@openrobotics.org

Add search via find file
354cd09 
Signed-off-by: John Shepherd <john@openrobotics.org>
@JShep1 JShep1 self-assigned this Oct 7, 2020
@github-actions github-actions bot added 🏰 citadel Ignition Citadel 📜 blueprint Ignition Blueprint 🔮 dome Ignition Dome labels Oct 7, 2020
@JShep1 JShep1 added the enhancement New feature or request label Oct 7, 2020
Add back removed space
a16cb88 
Signed-off-by: John Shepherd <john@openrobotics.org>
@JShep1 JShep1 marked this pull request as ready for review October 13, 2020 19:14
@JShep1 JShep1 requested a review from mjcarroll as a code owner October 13, 2020 19:14
mjcarroll
mjcarroll reviewed Oct 13, 2020
View reviewed changes
Copy link
Contributor

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

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

Are there any tests you could add here for this?

@JShep1
Copy link
Author

JShep1 commented Oct 13, 2020

Are there any tests you could add here for this?

Added in 7b02ea6

@JShep1 JShep1 requested a review from mjcarroll October 13, 2020 21:27
@mjcarroll
Copy link
Contributor

Looks like your new test is failing CI.

@JShep1
Copy link
Author

JShep1 commented Oct 14, 2020

Looks like your new test is failing CI.

Actually had my test passing but made an old test fail 😅 , I fixed it so it should be good now.

@codecov
Copy link

codecov bot commented Oct 14, 2020 •
edited
Loading

Codecov Report

Merging #102 into ign-common3 will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##           ign-common3     #102   +/-   ##
============================================
  Coverage        73.96%   73.97%           
============================================
  Files               69       69           
  Lines             9418     9421    +3     
============================================
+ Hits              6966     6969    +3     
  Misses            2452     2452
Impacted Files Coverage Δ
graphics/src/Material.cc 96.77% <100.00%> (+0.07%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 966a833...118ef09. Read the comment docs.

mjcarroll
mjcarroll reviewed Oct 14, 2020
View reviewed changes
graphics/src/Material.cc Outdated Show resolved Hide resolved
@JShep1 JShep1 force-pushed the jshep1/add_texture_uris branch 6 times, most recently from 7617799 to a16cb88 Compare October 14, 2020 20:30
John Shepherd added 2 commits October 14, 2020 13:30
Merge branch 'ign-common3' into jshep1/add_texture_uris
016d4c8
Add test
fd3d578 
Signed-off-by: John Shepherd <john@openrobotics.org>
@JShep1 JShep1 requested a review from mjcarroll October 14, 2020 21:00
Add back spaces
755f802 
Signed-off-by: John Shepherd <john@openrobotics.org>
@mjcarroll mjcarroll changed the title Support fuel URLs for textures [ign-common3] Support fuel URLs for textures Oct 14, 2020
mjcarroll
mjcarroll reviewed Oct 16, 2020
View reviewed changes
Copy link
Contributor

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

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

Sorry, one more thing, we should be using joinPaths where possible.

The Windows CI failure is flaky test, I believe, as it is unrelated to this change.

graphics/src/Material.cc Outdated Show resolved Hide resolved
John Shepherd added 2 commits October 16, 2020 10:10
Add join paths
21bc311 
Signed-off-by: John Shepherd <john@openrobotics.org>
update to joinpaths
f4303be 
Signed-off-by: John Shepherd <john@openrobotics.org>
mjcarroll
mjcarroll approved these changes Oct 16, 2020
View reviewed changes
Copy link
Contributor

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

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

LGTM with green CI

@JShep1
Copy link
Author

JShep1 commented Oct 16, 2020

LGTM with green CI

Replied on one of your comments but commenting here too in case it gets swallowed: You might want to double check my updates to the tests, specifically for /path being changed to path, is there a way of specifying root, prepending / with common?

@JShep1
Copy link
Author

JShep1 commented Oct 16, 2020

@osrf-jenkins run tests please

@mjcarroll
Copy link
Contributor

You might want to double check my updates to the tests, specifically for /path being changed to path, is there a way of specifying root, prepending / with common?

You should be able to do joinPaths("/path", "foo", "bar", "baz")

Add root slash back
118ef09 
Signed-off-by: John Shepherd <john@openrobotics.org>
@mjcarroll
Copy link
Contributor

ignition_common-ci-pr_any-ubuntu_auto-amd64 and ignition_common-abichecker-any_to_any-ubuntu_auto-amd64 were infra issues. I think this is probably safe to merge.

@JShep1
Copy link
Author

JShep1 commented Oct 16, 2020

@osrf-jenkins run tests please

@mjcarroll
Copy link
Contributor

Infra issues again, I'm going to go ahead and merge.

@mjcarroll mjcarroll merged commit b9f93ed into ign-common3 Oct 16, 2020
@mjcarroll mjcarroll deleted the jshep1/add_texture_uris branch October 16, 2020 20:39
mjcarroll pushed a commit that referenced this pull request Oct 19, 2020
@mjcarroll
[ign-common3] Support fuel URLs for textures (#102)
3dfd58e 
Resolves gazebosim/gz-sim#343

Signed-off-by: John Shepherd <john@openrobotics.org>
mjcarroll pushed a commit that referenced this pull request Oct 19, 2020
@mjcarroll
[ign-common3] Support fuel URLs for textures (#102)
f5ee75a 
Resolves gazebosim/gz-sim#343

Signed-off-by: John Shepherd <john@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
mjcarroll pushed a commit that referenced this pull request Oct 29, 2020
@mjcarroll
[ign-common3] Support fuel URLs for textures (#102)
b7a042d 
Resolves gazebosim/gz-sim#343

Signed-off-by: John Shepherd <john@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mjcarroll mjcarroll mjcarroll approved these changes

Assignees

@JShep1 JShep1

Labels
📜 blueprint Ignition Blueprint 🏰 citadel Ignition Citadel 🔮 dome Ignition Dome enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@JShep1 @mjcarroll