Skip to content

Conversation

laurentmuller
Copy link
Contributor

@laurentmuller laurentmuller commented Aug 13, 2025

Fix #1526

@laurentmuller laurentmuller changed the title First PR for #1526. PR for #1526. Aug 13, 2025
@garak garak changed the title PR for #1526. Set default upload destination Aug 14, 2025
Copy link
Collaborator

@garak garak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs need update

@laurentmuller
Copy link
Contributor Author

Hi,
What I need to update ?
Just remove the upload_destination entry in docs/usage.md and explain how the default value is computed?

@garak
Copy link
Collaborator

garak commented Aug 18, 2025

Hi, What I need to update ? Just remove the upload_destination entry in docs/usage.md and explain how the default value is computed?

The option can't be removed, it's still valid. The explanation is needed though

@laurentmuller
Copy link
Contributor Author

After PR #1529, the ContainerInterface argument is no more present in PropertyMappingResolver class.
It is so impossible to get the 'kernel.project_dir' property.
How to resolve that ?

@garak
Copy link
Collaborator

garak commented Aug 26, 2025

With a proper injection

@laurentmuller
Copy link
Contributor Author

Done!

docs/usage.md Outdated
@@ -41,6 +41,10 @@ vich_uploader:
This is the minimal amount of configuration needed in order to describe a
working mapping.

> [!NOTE]
> If the `upload_destination` parameter is missing, it this concatenated with
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"it this concatenated" doesn't make sense

@@ -200,6 +201,19 @@ protected function fixDbDriverConfig(array $config): array
return $config;
}

protected function fixUploadDestinationConfig(ContainerBuilder $container, array $config): array
{
// mapping with no declared upload_destination use the uri_prefix
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"use" -> "uses"

Copy link
Contributor Author

@laurentmuller laurentmuller Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the same as comment in fixDbDriverConfig() function. We must also update?

docs/usage.md Outdated
@@ -41,6 +41,10 @@ vich_uploader:
This is the minimal amount of configuration needed in order to describe a
working mapping.

> [!NOTE]
> If the `upload_destination` parameter is missing, it this set automatically
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not much better than before. Don't you have a spell checker?

@garak garak merged commit d811b78 into dustin10:master Aug 27, 2025
12 checks passed
@laurentmuller
Copy link
Contributor Author

Any chance to create a new version ?

@garak
Copy link
Collaborator

garak commented Aug 29, 2025

Any chance to create a new version ?

As soon as we solve 1531

@laurentmuller
Copy link
Contributor Author

OK. Thank You.

garak pushed a commit to garak/VichUploaderBundle that referenced this pull request Aug 30, 2025
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.

Omit default upload destination.
2 participants