Include support for Windows in 1.0.0 #312
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ahcorde
force-pushed
the
ahcorde/rolling/1.0.0_windows
branch
from
924a3b1 to
73613dd
Compare
rcl
rcl_action✅ rcl_lifecycle✅ |
|
@ahcorde could we rebase this to target |
Yadunund
requested changes
Dec 9, 2024
Comment on lines
81
to
86
| // z_owned_string_t shm_enabled; | ||
| // zc_config_get_from_str(z_loan(config), Z_CONFIG_SHARED_MEMORY_KEY, &shm_enabled); | ||
| // auto always_free_shm_enabled = rcpputils::make_scope_exit( | ||
| // [&shm_enabled]() { | ||
| // z_drop(z_move(shm_enabled)); | ||
| // }); |
There was a problem hiding this comment.
Does the code not compile if uncommented on Windows?
Instead of commenting it for all users, can we rely on #ifdef blocks?
Yadunund
force-pushed
the
ahcorde/rolling/1.0.0_windows
branch
2 times, most recently
from
e734f05 to
5e5836c
Compare
ahcorde
force-pushed
the
ahcorde/rolling/1.0.0_windows
branch
from
5e5836c to
3b6fb80
Compare
Signed-off-by: Alejandro Hernandez Cordero <[email protected]> Removed some warnings Signed-off-by: Alejandro Hernandez Cordero <[email protected]> Removed C4099 Windows warning Signed-off-by: Alejandro Hernandez Cordero <[email protected]> Support Windows Signed-off-by: Yadunund <[email protected]> undo changes Signed-off-by: Yadunund <[email protected]>
Yadunund
force-pushed
the
ahcorde/rolling/1.0.0_windows
branch
from
3b6fb80 to
26360a8
Compare
Signed-off-by: Alejandro Hernandez Cordero <[email protected]>
Signed-off-by: Alejandro Hernandez Cordero <[email protected]>
Yadunund
approved these changes
Dec 9, 2024
Just so we don't have to use as many workarounds. Signed-off-by: Chris Lalancette <[email protected]>
clalancette
approved these changes
Dec 11, 2024
Yadunund
reviewed
Dec 11, 2024
| ///============================================================================= | ||
| class rmw_context_impl_s final | ||
| struct rmw_context_impl_s final |
There was a problem hiding this comment.
@clalancette was this change to struct intentional?
There was a problem hiding this comment.
There is a warning on Windows, rmw_context_impl_s is defined in the rmw as a struct but we are implementing this as a class.
There was a problem hiding this comment.
@clalancette was this change to struct intentional?
Yes, because otherwise Windows complains that it is defined as a struct in rmw.h, but as a class here.
There was a problem hiding this comment.
I see. Thanks for pointing that out.
This was referenced Dec 11, 2024
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For visibility reason I created a branch called dev/1.0.0 in this repository, otherwise I can't open the PR
Include support for Windows in 1.0.0