Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
file_prepare_directory() was replaced with \Drupal::service('file_system')->prepareDirectory(). See [#3006851]. But this is only for Drupal 8.7. So the fix for #3042934: Drupal 9 Deprecated Code Report makes the module incompatible with Drupal lower than 8.7.
Comment | File | Size | Author |
---|---|---|---|
#23 | interdiff_19_20.txt | 359 bytes | Spokje |
#23 | 3084611_indicate_87_requirement-20.patch | 3.17 KB | Spokje |
#23 | 2019-10-02_19-25-09.png | 11.96 KB | Spokje |
#19 | 3084611_indicate_87_requirement-19.patch | 3.11 KB | greggles |
#15 | 3084611_indicate_87_requirement-15.patch | 657 bytes | greggles |
Comments
Comment #2
mfernea CreditAttribution: mfernea at AmeXio commentedMy solution would be to add into composer.json:
Comment #3
geek-merlin...plus the same condition in .info.yml.
Patch much appreciated!
Comment #4
gregglesI think why it breaks is important ;)
Comment #5
jason_purdy CreditAttribution: jason_purdy commentedThis just bit me. :( So I guess until this is addressed, those of us with Drupal 8.6, we need to pin stage_file_proxy to 1.0-beta1.
Comment #6
gregglesLike this?
Comment #7
geek-merlinI think we should also add that .info.yml condition for the folks that don't use composer.
Comment #8
geek-merlinAh, also the patch contains a bogus comma, which makes it invalid json.
Comment #9
gregglesThis composer should parse :)
I added the change to the info.yml but that is only supported since 8.7.7 so it doesn't really help for this situation, right?
Comment #10
gregglesI also added a warning to the project page.
Comment #11
mfernea CreditAttribution: mfernea at AmeXio commented@jason_purdy Yes. At least this is what I did.
@greggles I think a message here https://www.drupal.org/project/stage_file_proxy/releases/8.x-1.0-rc1 would also help.
Comment #12
gregglesGood call, I've done that now.
Comment #13
mfernea CreditAttribution: mfernea at AmeXio commentedI noticed this in [#3070687].
So we only need to alter the composer.json file.
Comment #14
mfernea CreditAttribution: mfernea at AmeXio commentedComment #15
gregglesGreat catch.
Comment #17
gregglesIt says it "failed testing" but also the build details show "08:38:42 Finished: SUCCESS" on the jenkins console. I guess that was an issue b/c I changed the testing to focus on PHP 7.2 instead of php 5.6 (5.6 tests are failing now due to a composer issue and honestly php 5.6 is quite old at this point).
Comment #19
gregglesI didn't realize, but it seems there are no tests and when that's the case the build will be successful and yet the status gets moved to "needs work".
Here's an updated patch with a few code style fixes as well.
Anyone care to give a RTBC?
Comment #21
geek-merlinI can confirm that the changes in composer.json and stage_file_proxy.info.yml make sense.
It looks like the rest is unrelated changes like whitespace fixes, which make sense but would suggest moving to separate commit.
I did not test that composer installs successfully (and afaik for composer reasons testbot/composer-patches is not the same).
But as such a test can only be done after a commit, let's do it and in the unlikely case we can fix that before a release.
Comment #22
geek-merlin;-)
Comment #23
Spokje- Patch #19 applies cleanly to the latest dev (
'dev-1.x#77880c3dccf30160541a270c82d27672b5a5be26'
).- Code Style fixes all make sense to me.
- Tested on Drupal 8.7.7 and all worked for me.
Just one small nitpick:
I've added the lines
to
stage_file_proxy.info.yml
This will prevent users from enabling the module in the GUI when on Core < 8.7.x (Drush will still install it).
Screenshot is from a D8.6.17 site, checkbox to enable is greyed out.
Comment #25
gregglesThanks, @geek.merlin - all helpful!
Nice idea, @spokje!
Now committed/pushed.
I think we should make a new release as well in the very near future.
Comment #26
Spokje+1 from me on that. :)
Comment #27
SpokjeAnd a -3 for accidentally changing the status :(
Comment #28
gregglesCool.
I filed #3085297: Release 8.x-1.0-rc2.
Comment #29
gregglesIn case anyone here is curious, this issue introduced a regression #3085394: 'core_version_requirement' issue with Core 8.7.8. Now fixed.