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 -> gz : Release Tools Migrate Internal Vars #753

Merged
merged 7 commits into from
Aug 18, 2022
Merged

ign -> gz : Release Tools Migrate Internal Vars #753

merged 7 commits into from
Aug 18, 2022

Conversation

methylDragon
Copy link
Contributor

Part of: #746

@methylDragon methylDragon mentioned this pull request Jun 24, 2022
Closed
@chapulina chapulina added the ign to gz Renaming Ignition to Gazebo. label Jun 24, 2022
@methylDragon
Copy link
Contributor Author

Ah, I remember why there are many duplications here. I had a lot of conflicts if I didn't base the branch off the project name migration. I think we should merge the name migration first, then rebase this! Otherwise we'll have a lot of conflicts for the project name migration.

@j-rivero
Copy link
Contributor

j-rivero commented Aug 3, 2022

Ah, I remember why there are many duplications here. I had a lot of conflicts if I didn't base the branch off the project name migration. I think we should merge the name migration first, then rebase this! Otherwise we'll have a lot of conflicts for the project name migration.

Is there anything missing from the name migration that is blocking this PR? I'm happy to help to unblock this one. Thanks.

@methylDragon
Copy link
Contributor Author

Ah, I remember why there are many duplications here. I had a lot of conflicts if I didn't base the branch off the project name migration. I think we should merge the name migration first, then rebase this! Otherwise we'll have a lot of conflicts for the project name migration.

Is there anything missing from the name migration that is blocking this PR? I'm happy to help to unblock this one. Thanks.

I think there aren't any blockers here, we just weren't sure about merging before feature freeze since it could've broken stuff :o
Let me resolve the conflicts and then you can give a look-see to see if what I've changed makes sense?

j-rivero
j-rivero reviewed Aug 9, 2022
View reviewed changes
jenkins-scripts/ign_common-default-devel-windows-amd64.bat
@@ -4,13 +4,13 @@ set VCS_DIRECTORY=gz-common
set PLATFORM_TO_BUILD=x86_amd64
set IGN_CLEAN_WORKSPACE=true

set DEPEN_PKGS=assimp ffmpeg freeimage gts tinyxml2
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this removal an error or there is a reason to remove it in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Humm, this might've happened when I rebased, I think it's an error, I'll revert the change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

d513c8d

Copy link
Contributor

Choose a reason for hiding this comment

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

it looks like assimp is gone again. can you restore it?

Copy link
Contributor

Choose a reason for hiding this comment

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

good catch! 6138403

@j-rivero j-rivero mentioned this pull request Aug 16, 2022
Merged
j-rivero and others added 2 commits August 17, 2022 01:21
@j-rivero
Windows fixes for 753 (ign -> gz) (#792)
10e9e12 
* Reverse the logic for package names
* Remove redundant code. Always called from colcon which has the same logic
@scpeters
Merge branch 'master' into migrate_internal_vars
e18ced7
@scpeters
Copy link
Contributor

testing windows script changes:

scpeters
scpeters requested changes Aug 17, 2022
View reviewed changes
jenkins-scripts/ign_common-default-devel-windows-amd64.bat
@@ -4,13 +4,13 @@ set VCS_DIRECTORY=gz-common
set PLATFORM_TO_BUILD=x86_amd64
set IGN_CLEAN_WORKSPACE=true

set DEPEN_PKGS=assimp ffmpeg freeimage gts tinyxml2
Copy link
Contributor

Choose a reason for hiding this comment

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

it looks like assimp is gone again. can you restore it?

@scpeters
Copy link
Contributor

I was going to start a test build of ign-gazebo6 on windows, but that has been broken since gazebosim/gz-sim#1626

@j-rivero j-rivero merged commit d2b2278 into master Aug 18, 2022
@j-rivero j-rivero deleted the migrate_internal_vars branch August 18, 2022 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chapulina chapulina chapulina requested changes

@j-rivero j-rivero j-rivero left review comments

@scpeters scpeters scpeters approved these changes

Assignees
No one assigned
Labels
ign to gz Renaming Ignition to Gazebo.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@methylDragon @j-rivero @scpeters @chapulina