search

kevinkhill / lavacharts

Lavacharts is a graphing / charting library for PHP 5.4+ that wraps Google's Javascript Chart API.
http://lavacharts.com
Other
620 stars 142 forks source link

Fix psr4 autoload warning #322

Closed ChaerilM closed 3 years ago

ChaerilM commented 4 years ago

composer gives warning while update. in some case, it does not add lavachart to autoloading due to this issue

coveralls commented 4 years ago

Coverage Status

Coverage remained the same at 69.207% when pulling bed74ac873046940feab5e6132216ac6b985f7c7 on Zabanya:3.1 into e3b719c033567d967983f38e0d4e8e6efb11d502 on kevinkhill:3.1.

ChaerilM commented 4 years ago

i could not find any other file that uses Khill\Lavacharts\Support\Renderable nor Khill\Lavacharts\Support\RenderableTrait

kevinkhill commented 4 years ago

So I have a useless trait causing issues... I will give a second look when I get a chance. Thank you for bringing this to my attention

ChaerilM commented 4 years ago

it might be an uncleaned commit after migration since Khill\Lavacharts\Support\Renderable and Khill\Lavacharts\Support\Traits\RenderableTrait had similar structure

kevinkhill commented 4 years ago

It actually looks like you uncovered my indecisiveness in choosing inheritance over composition with traits... I think Renderable used to be a trait, but then I made it a class...

investigating...

kevinkhill commented 4 years ago

I also moved the 4.0 work into its own branch so master tracks the current development of 3.1 again

yaayes commented 3 years ago

Hi thanks for the PR, but I still see this warning when running composer update

Class Khill\Lavacharts\Support\RenderableTrait located in ./vendor/khill/lavacharts/src/Support/Renderable.php does not comply with psr-4 autoloading standard. Skipping.
yaayes commented 3 years ago

Can you please tag a patch release to resolve this issue, thank you! cc @kevinkhill https://github.com/kevinkhill/lavacharts/compare/3.1.13..3.1

yaayes commented 3 years ago

I forget to mention that I have Composer version 2 (moved to it because it's much faster), so this is an issue now not just a warning!

ChaerilM commented 3 years ago

@yaayes I believe he push the patch for version 4, I've tried it but seems there was a breaking changes and I could not find doc for it. for now probably wait in indefinite time

kevinkhill commented 3 years ago

awww, I don't like discouraging you and hearing "indefinite" I have 30 minutes right now, lets see if I can make something happen.

yaayes commented 3 years ago

Hi thankyl you @ChaerilM and @kevinkhill for the response. Why not a patch for the version 3, it will not break anything since as @ChaerilM says that trait is not used anywhere in the package

kevinkhill commented 3 years ago

https://github.com/kevinkhill/lavacharts/releases/tag/3.1.14

kevinkhill commented 3 years ago

let me know if errors arise please! I didn't test anything, because I'm off to my paying job now :smile:

yaayes commented 3 years ago

Thanks @kevinkhill for taking the time to solve this. Very appreciated!

yaayes commented 3 years ago

Updated successfully without any warnings related to composer v2, and I can confirm that nothing break (at least in my application which use like four/five type of commons charts)

kevinkhill commented 3 years ago

Awesome, glad to hear that. 🤞🏻 All my hot glue and duct tape keeps 3.1 together until I eveeeeentually carve time for v4