whikloj / BagItTools

A PHP BagIt library
MIT License
11 stars 5 forks source link

Ensure we start working with an absolute path #27

Closed whikloj closed 3 years ago

whikloj commented 3 years ago

Resolves #26

codecov[bot] commented 3 years ago

Codecov Report

Merging #27 (b480cbd) into main (94b6068) will increase coverage by 1.25%. The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main      #27      +/-   ##
============================================
+ Coverage     82.47%   83.72%   +1.25%     
- Complexity      481      484       +3     
============================================
  Files             7        7              
  Lines          1295     1303       +8     
============================================
+ Hits           1068     1091      +23     
+ Misses          227      212      -15     
Impacted Files Coverage Δ
src/Bag.php 81.06% <100.00%> (+1.97%) :arrow_up:
src/BagUtils.php 91.11% <100.00%> (+0.74%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 94b6068...b480cbd. Read the comment docs.

whikloj commented 3 years ago

@asmecher do you want to give this a try? I apologize that this simple change has come at the cost of removing PHP 7.2 support. I'll cut this as a ^3.0 release

whikloj commented 3 years ago

@asmecher I updated main to remove PHP 7.2 support and fix all the other underlying bits. So now this PR is just about the issue at hand. Please have a look and let me know if I am meeting the use-case.

asmecher commented 3 years ago

Thanks, @whikloj! I tested this by applying https://github.com/whikloj/BagItTools/pull/27.diff to my Composer-based installation (version 2.1.0). The patch applied successfully (with a bit of noise due to upstream changes):

patching file src/Bag.php
Hunk #3 succeeded at 523 with fuzz 2.
Hunk #4 succeeded at 1176 (offset 2 lines).
patching file src/BagUtils.php
Hunk #2 succeeded at 125 with fuzz 2.
Hunk #3 succeeded at 140 (offset 1 line).
patching file tests/BagInternalTest.php
patching file tests/BagTest.php
Hunk #1 succeeded at 18 with fuzz 1 (offset -1 lines).
Hunk #2 succeeded at 30 (offset -1 lines).
Hunk #3 succeeded at 94 (offset -1 lines).
Hunk #4 succeeded at 774 (offset -11 lines).
Hunk #5 succeeded at 968 (offset -19 lines).
patching file tests/BagUtilsTest.php
Hunk #1 succeeded at 75 (offset -1 lines).

I tested this with a relative path and it worked fine where it previously failed.

Recent releases of our software require PHP7.3, so the PHP7.2 loss isn't a problem.

Thanks for the quick turn-around on this, I appreciate it!

whikloj commented 3 years ago

I'll tag a 3.0.1 in the morning

asmecher commented 3 years ago

Thanks, @whikloj, much appreciated!