add setup script, rename prefix script into local_setup #38
Conversation
dirk-thomas
force-pushed
the
refactor_setup_files
branch
from
82d88d5
to
71f07d5
Compare
Codecov Report
@@ Coverage Diff @@
## master #38 +/- ##
==========================================
+ Coverage 77.61% 77.78% +0.17%
==========================================
Files 48 48
Lines 2519 2543 +24
Branches 400 405 +5
==========================================
+ Hits 1955 1978 +23
Misses 550 550
- Partials 14 15 +1
Continue to review full report at Codecov.
|
dirk-thomas
force-pushed
the
refactor_setup_files
branch
from
71f07d5
to
28e208b
Compare
colcon_core/shell/__init__.py
Outdated
| @@ -310,3 +310,40 @@ def create_environment_hook( | |||
| 'Could not find a primary shell extension for creating an ' | |||
| 'environment hook') | |||
| return hooks | |||
|
|
|||
|
|
|||
| _colcon_prefix_path_cache = {} | |||
There was a problem hiding this comment.
Why cache the result? Can the COLCON_PREFIX_PATH environment variable change between calls to get_colcon_prefix_path()?
There was a problem hiding this comment.
I tried to describe that in the docblock:
The result is being cached based on the passed skip parameter to avoid printing the warning on repeated invocations.
While the environment isn't supposed to change during one run of the tool the fact that the result would be invalid if the environment does change isn't really a good idea.
I refactored the logic in 6694243 to actually store which warning has already been logged and just use that global information to avoid repeating the warning. The computation happens every time and is therefore correct no matter what happens to the environment.
dirk-thomas
force-pushed
the
refactor_setup_files
branch
2 times, most recently
from
33b1d05
to
6694243
Compare
dirk-thomas
force-pushed
the
refactor_setup_files
branch
2 times, most recently
from
d828ae4
to
a9bf96d
Compare
There was a problem hiding this comment.
LGTM.
I didn't check the .sh code. I checked that sourcing setup.bat chained workspaces successfully using some python packages. In a fresh cmd instance sourcing setup.bat of the last workspace in a chain resulted in aPYTHONPATH containing all workspaces from the highest workspace to the lowest.
dirk-thomas
force-pushed
the
refactor_setup_files
branch
from
8809ed5
to
934b843
Compare
|
Ignoring failing CI which is due to a regression in the new |
While not complete (the
.batscripts still need to be refactored) I would like to get a first review / feedback if that is the desired result.