Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MediaBundle] Use parameter instead of hardcoded media path #2782

Merged
merged 1 commit into from
May 8, 2021

Conversation

dannyvw
Copy link
Contributor

@dannyvw dannyvw commented Oct 8, 2020

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Fixed tickets comma separated list of tickets fixed by the PR

Copy link

@ProfessorKuma ProfessorKuma left a comment

Choose a reason for hiding this comment

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

Hi @, your PR needs some changes

  • Your answer if this PR is a new feature seems to be incorrect.

Copy link

@ProfessorKuma ProfessorKuma left a comment

Choose a reason for hiding this comment

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

Hi @, your PR needs some changes

  • This PR seems to need a milestone of a minor release.

@ProfessorKuma ProfessorKuma added this to the 5.7.0 milestone Oct 8, 2020
Copy link

@ProfessorKuma ProfessorKuma left a comment

Choose a reason for hiding this comment

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

Hi @, your PR passed all our requirements.

Thank you for contributing!

Copy link
Member

@acrobat acrobat left a comment

Choose a reason for hiding this comment

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

Small change needed, but other than that it looks good!

src/Kunstmaan/MediaBundle/Helper/File/FileHelper.php Outdated Show resolved Hide resolved
@dannyvw dannyvw force-pushed the media-path branch 2 times, most recently from 2a8a51f to 858b145 Compare October 13, 2020 08:14
@acrobat acrobat modified the milestones: 5.7.0, 5.8.0 Oct 21, 2020
@dannyvw
Copy link
Contributor Author

dannyvw commented Mar 12, 2021

@acrobat Do you have an update for this?

@acrobat acrobat modified the milestones: 5.8.0, 5.9.0 Mar 17, 2021
@acrobat
Copy link
Member

acrobat commented Apr 25, 2021

@dannyvw I'm not allowed to push extra fixes to this branch so can you check/apply these changes and rebase againt the current master? Then we should be go to go for the merge!

commit ef70ba16918f457692e60e9da8174f7de75e11bd
Author: Jeroen Thora <jeroenthora@gmail.com>
Date:   Sun Apr 25 20:48:18 2021 +0200

    Extra fixes

diff --git a/UPGRADE-5.9.md b/UPGRADE-5.9.md
new file mode 100644
index 000000000..b15611ff7
--- /dev/null
+++ b/UPGRADE-5.9.md
@@ -0,0 +1,7 @@
+UPGRADE FROM 5.8 to 5.9
+=======================
+
+MediaBundle
+------------
+
+* Not passing a value for the "$mediaPath" parameter of "\Kunstmaan\MediaBundle\Helper\File\FileHelper::__construct" is deprecated, a value will be required.
diff --git a/src/Kunstmaan/MediaBundle/Helper/File/FileHelper.php b/src/Kunstmaan/MediaBundle/Helper/File/FileHelper.php
index d0e0778e0..2b2d4dbf6 100644
--- a/src/Kunstmaan/MediaBundle/Helper/File/FileHelper.php
+++ b/src/Kunstmaan/MediaBundle/Helper/File/FileHelper.php
@@ -28,18 +28,20 @@ class FileHelper
      */
     protected $path;
 
-    protected $mediaPath;
+    /** @var string */
+    private $mediaPath;
 
     /**
-     * @param Media  $media
-     * @param string $mediaPath
+     * @param Media       $media
+     * @param string|null $mediaPath
      */
     public function __construct(Media $media, string $mediaPath = null)
     {
         $this->media = $media;
 
         if ($mediaPath === null) {
-            @trigger_error(sprintf('Not passing the media path as the second argument of "%s" is deprecated since KunstmaanMediaBundle 5.7 and will be required in KunstmaanMediaBundle 6.0. Injected the required parameter in the constructor instead.', __METHOD__), E_USER_DEPRECATED);
+            @trigger_error(sprintf('Not passing a value for the "$mediaPath" parameter of "%s" is deprecated since KunstmaanMediaBundle 5.9 and a value will be required in KunstmaanMediaBundle 6.0. Injected the required parameter in the constructor instead.', __METHOD__), E_USER_DEPRECATED);
+
             $mediaPath = '/uploads/media/';
         }

@acrobat acrobat merged commit cabf34d into Kunstmaan:master May 8, 2021
@acrobat
Copy link
Member

acrobat commented May 8, 2021

Thanks @dannyvw!

@dannyvw dannyvw deleted the media-path branch May 9, 2021 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants