Backport #561: Use common::setenv #666
Conversation
Signed-off-by: Louise Poubel <[email protected]>
Codecov Report
@@ Coverage Diff @@
## ign-gazebo3 #666 +/- ##
============================================
Coverage 77.88% 77.88%
============================================
Files 210 210
Lines 11648 11649 +1
============================================
+ Hits 9072 9073 +1
Misses 2576 2576
Continue to review full report at Codecov.
|
| _putenv(sstr.str().c_str()); | ||
| } | ||
| #ifdef _MSC_VER | ||
| # define popen _popen |
There was a problem hiding this comment.
I know this is a backport, but are these lines related with these changes?
There was a problem hiding this comment.
Good point, the removal of the custom setenv functions goes well with the rest of the changes, but the popen defines are not directly related.
Since this was in the original PR, I'm leaning towards leaving it here to decrease the diff between branches. It shouldn't really affect ign-gazebo3 because it's not supported on Windows.
🦟 Bug fix
Summary
Backporting #561.
Similar to #665, the motivation is to make forward-porting changes easier. On #622 we had to make lots of changes for the code ported from Dome to Edifice to work on Windows. If we encourage proper usage of
common::setenvfrom Citadel, new plugins should use correct boilerplate from the beginning.Checklist
codecheckpassed (See contributing)Note to maintainers: Remember to use Squash-Merge