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

Revert PR #330 "Add QoS profiles field to metadata struct" #334

Merged
merged 1 commit into from
Mar 27, 2020
Merged

Revert PR #330 "Add QoS profiles field to metadata struct" #334

merged 1 commit into from
Mar 27, 2020

Conversation

emersonknapp
Copy link
Collaborator

This reverts #330 due to broken windows build

Signed-off-by: Emerson Knapp emerson.b.knapp@gmail.com

@emersonknapp
Revert "Add QoS profiles field to metadata struct and provide seriali…
d84f8c3 
…zation utilities (#330)"

This reverts commit 5810588.

Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
@emersonknapp emersonknapp mentioned this pull request Mar 27, 2020
Merged
jacobperron
jacobperron approved these changes Mar 27, 2020
View reviewed changes
Copy link
Member

@jacobperron jacobperron 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

@emersonknapp
Copy link
Collaborator Author

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@emersonknapp
Copy link
Collaborator Author

Failed to vcs import urdf... seems confusing. Retrying

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@jacobperron
Copy link
Member

@emersonknapp I think it's a typo in the repos file

@jacobperron
Copy link
Member 

- url: https`://github.com/ros2/urdf.git
+ url: https://github.com/ros2/urdf.git

@emersonknapp
Copy link
Collaborator Author

Yeah, don't know where that came from

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@Karsten1987
Copy link
Collaborator

I understand that reverting would resolve the broken windows CI, but wouldn't it make sense to actually address the issue directly here? I mean, we have to run CI anyway.

@emersonknapp
Copy link
Collaborator Author

I mean, we have to run CI anyway.

I guess you're right, but it's really unfortunate that this is true. It should be easy to revert breaking changes.

@jacobperron
Copy link
Member

jacobperron commented Mar 27, 2020 •
edited
Loading

If this gets resolved, then I can continue with with ros2/launch#346 😉

Edit: And also the nightly build which will kick off in less than an hour.

@emersonknapp
Copy link
Collaborator Author

This CI run is almost done, when the windows build passes, let's merge the revert. I'd rather be trigger happy on reverts than on new changes.

@emersonknapp
Copy link
Collaborator Author

Green, merging. I'll reopen the PR and run CI ;) my bad, everyone

@emersonknapp emersonknapp merged commit 78ffcd7 into master Mar 27, 2020
@delete-merged-branch delete-merged-branch bot deleted the emersonknapp/revert-330 branch March 27, 2020 05:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jacobperron jacobperron jacobperron approved these changes

@Karsten1987 Karsten1987 Awaiting requested review from Karsten1987

@dirk-thomas dirk-thomas Awaiting requested review from dirk-thomas

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@emersonknapp @jacobperron @Karsten1987