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

Removing piwik composer deprecations #16006

Merged
merged 1 commit into from Jun 24, 2020
Merged

Conversation

tassoman
Copy link
Contributor

@tassoman tassoman commented Jun 1, 2020

Composer was warning about old naming convention

@Findus23
Copy link
Member

Findus23 commented Jun 2, 2020

Doing this would be a breaking change as it would update the plugins to the ones where the classes are called Matomo (I think), so it can only be done in Matomo 4 (and has already been done there)

@tsteur
Copy link
Member

tsteur commented Jun 2, 2020

@Findus23 would this be also the case when the same versions are still in use? It's generally not needed to fix this though in 3.x-dev indeed since it's already fixed in 4.x-dev.

@sgiehl
Copy link
Member

sgiehl commented Jun 3, 2020

It actually should work to merge this one. As we used the same repositories for both packages (before and after rename), all releases are actually available in both. That means older versions still have the older namespace...

@Findus23
Copy link
Member

Findus23 commented Jun 3, 2020

all releases are actually available in both

In that case it should work. I always assumed there would be no more piwik/ releases and only the newer versions in matomo/ releases.

@tassoman
Copy link
Contributor Author

tassoman commented Jun 3, 2020

I functionally tested my branch by browsing matomo locally after library update and didn't broke... Can't say if it's enough. Maybe I should do a test-run for a trustworthy check?

In my opinion, deprecation warnings in a production environment (3.x) it's a real warning.
I mean, if matomo/* is marked for development (4.x) shouldn't be flagged deprecating (piwik/*)
So what shall we do? 🤔

@tsteur tsteur added the Needs Review PRs that need a code review label Jun 3, 2020
@tsteur
Copy link
Member

tsteur commented Jun 3, 2020

I'd be happy to merge with the updated component names. The problem is that there are several version updates in the composer lock which I'm not keen to merge right now as we have already released an RC.

We might be able to merge this after the next release as these library upgrades could break something. There are some tests failing also but not 100% sure this is related to it.

@tsteur tsteur added this to the 3.13.7 milestone Jun 3, 2020
@tsteur
Copy link
Member

tsteur commented Jun 24, 2020

Note: Was going to merge but seems some tests are failing. Most tests because of other reasons but need to check why some of the other ones fail before merging.

@tsteur
Copy link
Member

tsteur commented Jun 24, 2020

Seems eg https://travis-ci.org/github/matomo-org/matomo/jobs/693471575#L898-L900 is failing
https://travis-ci.org/github/matomo-org/matomo/jobs/693471576#L987-L989 is failing as well.

Likely because the PDF lib was updated. Will probably need to fix these tests after merging.

Not sure if https://builds-artifacts.matomo.org/matomo-org/matomo/3.x-dev/40735/Dashboard_widget_move.png is caused by this PR or if it's random test failure.

@sgiehl
Copy link
Member

sgiehl commented Jun 24, 2020

Not sure if https://builds-artifacts.matomo.org/matomo-org/matomo/3.x-dev/40735/Dashboard_widget_move.png is caused by this PR or if it's random test failure.

That's a random failure I've seen before already. IIRC it is already fixed on 4.x-dev branch

@tsteur tsteur merged commit 51d6fe7 into matomo-org:3.x-dev Jun 24, 2020
@tsteur tsteur mentioned this pull request Jun 24, 2020
@tassoman tassoman deleted the piwik_depr branch July 13, 2020 12:01
@tassoman
Copy link
Contributor Author

Thank you team 👍

jonasgrilleres pushed a commit to 1024pix/pix-analytics that referenced this pull request Sep 22, 2020
jbuget pushed a commit to 1024pix/pix-analytics that referenced this pull request Sep 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants