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

Updates mp3 browser for more recent PHP versions (7.4, 8.x) #92

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jhalexand
Copy link

I went through and changed all the uses of curly branches as array index operators to brackets so it would work in newer PHP versions. I also fixed an issue where my absolute paths to mp3 directories were getting an extra "/" at the start of the path and messing up the URLs. I have a regex remove a double slash ("//") at the start of the folder path if there is one. (I did not root-cause this issue)

@Halison
Copy link

Halison commented Aug 29, 2024

I did the upgrade to joomla5, how can I share the code

@jhalexand
Copy link
Author

I haven't checked, but I would expect my changes to allow this to work in Joomla 5. You can check out my forked repo at https://github.com/jhalexand/mp3-browser/tree/master (as linked above) and build it with maven as described in the original author's README. If I have some time, I'll verify about J5 and see if there's anything in the metadata that needs to be updated.

@Halison
Copy link

Halison commented Aug 29, 2024

I mean, I did the upgrade and commited here https://github.com/Halison/mp3-browser

I tested and it is working well on joomla 5

@jhalexand
Copy link
Author

Ah, I misunderstood your comment! Thanks for moving it to Joomla 5! (I'll come back for a copy when I finally get my site updated to J5.) Since this pull request has sat open for so long, I assume the original author has abandoned this project. Maybe consider publishing it directly in the Joomla! Extensions Directory, giving credit to the original author for the basis of the work? You could probably update your README.md to reference this repo (it looks like maybe your repo isn't forked from this one?).

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.

2 participants