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

PHP7 compatibility: rename sparkline constructors to __construct #8410

Closed
wants to merge 1 commit into from
Closed

Conversation

energynumbers
Copy link
Contributor

Old naming method for constructors (same name as class) is deprecated as
of PHP7

Old naming method for constructors (same name as class) is deprecated as
of PHP7
@sgiehl
Copy link
Member

sgiehl commented Jul 24, 2015

As that is a third party lib, we should not do any changes without mentioning them in README.md

Btw. in my opinion it would be a better way to find a good alternative to the old sparkline library we are using. Maybe we could also write a short implementation for our needs, and remove that external lib.

@tsteur
Copy link
Member

tsteur commented Jul 27, 2015

If that sparkline lib is no longer maintained, looking for a new one sounds good 👍

@jcracknell
Copy link

How about generating an <svg:path>?

@sgiehl
Copy link
Member

sgiehl commented Aug 5, 2015

svg is an option for sure, but we still need to be able to generate them as an image. Otherwise we can't use them in email/pdf reports and the mobile app.

@mattab mattab changed the title rename sparkline constructors to __construct PHP7 compatibility: rename sparkline constructors to __construct Aug 9, 2015
@mattab mattab added this to the 2.15.0 milestone Aug 9, 2015
@mattab
Copy link
Member

mattab commented Aug 9, 2015

Targeting to 2.15.0 as it would be appreciated for users if we support PHP7 in Piwik 2.X branch 👍

@mattab
Copy link
Member

mattab commented Aug 9, 2015

As that is a third party lib, we should not do any changes without mentioning them in README.md

Thanks @energynumbers for the pull request. do you mind adding a notes in libs/README.md about your modification? then we can merge the pull request, cheers

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.

None yet

5 participants