Skip to content

Set files path as a static path for concrete5 driver#514

Merged
mattstauffer merged 2 commits intolaravel:masterfrom
KorvinSzanto:patch-3
Dec 1, 2022
Merged

Set files path as a static path for concrete5 driver#514
mattstauffer merged 2 commits intolaravel:masterfrom
KorvinSzanto:patch-3

Conversation

@KorvinSzanto
Copy link
Copy Markdown
Contributor

Previously, testing a concrete5 site that had tons of 404s and required file based sessions would force each request to load sequentially causing the site to load pretty slowly. This change makes it so that the driver is aware of the files directory and markes anything within as a static file.

Previously testing a concrete5 site that had tons of 404s and required file based sessions would force each request to load sequentially causing the site to load pretty slowly. This change makes it so that the driver is aware of the files directory and markes anything within as a static file.
@mattstauffer
Copy link
Copy Markdown
Collaborator

@KorvinSzanto Could you ask a few other concrete5 folks to check this out for you, and give us emoji responses so we can verify it's working for multiple folks? None of us use concrete5 so we can't check this.

@JohntheFish
Copy link
Copy Markdown

What is the specification on $uri ? Is it site relative, or host relative? If host relative, testing ===0 could have issues with installations in sub-folders.

@biplobice
Copy link
Copy Markdown

This helps me a lot for sites which have lots of 404s.
It works fine for me, and the installation in sub-folders didn't work, without this code as well.

@mattstauffer
Copy link
Copy Markdown
Collaborator

If we can get more concrete5 folks to check it, we could merge this, but it's been four years since I asked for more input so I'm gonna close it.

@KorvinSzanto KorvinSzanto deleted the patch-3 branch March 31, 2022 14:59
@KorvinSzanto
Copy link
Copy Markdown
Contributor Author

I did have folks weigh in as evidenced by the emoji responses so I'm not tracking on what was missing. I'm guessing you were hoping for more than 5 or so people?

@mattstauffer
Copy link
Copy Markdown
Collaborator

@KorvinSzanto To be honest, I was looking for comments and didn't even check the emoji responses :) Please feel free to re-make the PR if you'd like.

@KorvinSzanto KorvinSzanto restored the patch-3 branch March 31, 2022 17:26
@KorvinSzanto
Copy link
Copy Markdown
Contributor Author

I've restored the branch so this PR should be reopenable. If not I can go ahead and make a new PR

@mattstauffer mattstauffer reopened this Apr 1, 2022
@mattstauffer
Copy link
Copy Markdown
Collaborator

@KorvinSzanto Thanks! i just did a merge resolution; can you verify it still works correctly after that?

@mattstauffer mattstauffer merged commit 3ccbd0c into laravel:master Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants