Closed jrfnl closed 2 years ago
Aside from the typo causing problems in the test, this looks great to me. I think this is the right approach. Thanks!
I wonder if that's because it needs the executable bit. Do you have a way of setting chmod +x
on that file and committing that change?
I wonder if that's because it needs the executable bit. Do you have a way of setting
chmod +x
on that file and committing that change?
Ha! Yes. That was it! Might be useful to add that information to the CONTRIBUTING
file.
If it helps, even though file permissions are not handle the same on Windows, the git ls-files -s
command to check the status works and will show the correct chmod status. The permissions for a file can be changed on Windows by running this command: git update-index --chmod=+x <file>
.
I've updated the PR and have verified the new test fails without the fix and passes with the fix in place, so should be good to go now.
Thanks @ramsey for coaching me through this one.
Merging #213 (bf53470) into v2 (6e86502) will decrease coverage by
0.70%
. The diff coverage is0.00%
.
@@ Coverage Diff @@
## v2 #213 +/- ##
==========================================
- Coverage 88.70% 88.00% -0.71%
==========================================
Files 5 5
Lines 124 125 +1
==========================================
Hits 110 110
- Misses 14 15 +1
Impacted Files | Coverage Δ | |
---|---|---|
bin/composer_paths.sh | 87.80% <0.00%> (-2.20%) |
:arrow_down: |
Hmm.. curious that Codecov doesn't pick up on the new unit test...
Hmm.. curious that Codecov doesn't pick up on the new unit test...
I'm still learning how the Bash code coverage tooling works. 🤷🏻♂️
I have a suspicion that multi-line commands in Bash (like this one) aren't being correctly reported.
That might well explain it, though the second line of the command is recognized as covered (correctly).
[Edit]: on second look, no doesn't seem to be recognized either, so yeah, you're right. Multi-line commands seem to be the issue.
Description
As far as I can see, the
--no-check-lock
argument has been available since Composer 1.0, so adding this argument to thecomposer validate
command should be safe and should prevent the issues as reported in #206.Includes tests (though the test isn't running properly yet).
Motivation and context
Fixes #206
How has this been tested?
I've tested this locally with the plain command with both Composer 1 and 2, but have not tested this in the context of this action.
I've added a unit test, but haven't managed to get it running properly.
Types of changes
PR checklist