Skip to content

Conversation

@wpjscc
Copy link
Contributor

@wpjscc wpjscc commented Feb 19, 2023

when driver is cloud driver, the url is dependent on ASSET_URL。

it should dependent on cloud ENDPOINT

@what-the-diff
Copy link

what-the-diff bot commented Feb 19, 2023

  • The getPublicPath() method was changed to use the url() function of a disk object instead of Url::asset().

@bennothommo bennothommo added maintenance PRs that fix bugs, are translation changes or make only minor changes needs review Issues/PRs that require a review from a maintainer labels Feb 20, 2023
@bennothommo
Copy link
Member

I think there's a specific rationale for why it's like that - @jaxwilko or @LukeTowers might know more.

@jaxwilko
Copy link
Member

@bennothommo I vaguly remember it being important, not sure why though. In theory this change looks okay but maybe @LukeTowers knows more

@LukeTowers
Copy link
Member

Hmm, on my phone at the moment but check the git blame. Hopefully there's a decent commit message

@bennothommo
Copy link
Member

5aa351f

Not sure about that...

@LukeTowers LukeTowers added Status: Completed and removed needs review Issues/PRs that require a review from a maintainer labels Feb 20, 2023
@LukeTowers LukeTowers added this to the v1.2.2 milestone Feb 20, 2023
@LukeTowers
Copy link
Member

This is totally fine then @bennothommo, that commit is from before I added the getDisk() method to the File model in wintercms/storm@48fed9b, just leftover code from the old non-disk focused way of doing things.

@LukeTowers LukeTowers changed the title fix not local driver return url is error Return correct URLs from File->getPublicPath for non-local disks Feb 20, 2023
@LukeTowers LukeTowers merged commit 4ed9b10 into wintercms:develop Feb 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance PRs that fix bugs, are translation changes or make only minor changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants