@Findus23 opened this Pull Request on February 21st 2017 Member

fixes #11370

Let's see how many places I have missed.

@Findus23 commented on February 21st 2017 Member

Cloning into 'plugins/Morpheus/icons'...
Permission denied (publickey).

fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
Clone of 'git@github.com:piwik/device-icons.git' into submodule path 'plugins/Morpheus/icons' failed

Seems like travis can't clone the submodule

@sgiehl commented on February 21st 2017 Member

try using https://github.com/piwik/device-icons.git instead of git<a class='mention' href='https://github.com/github'>@github</a>.com:piwik/device-icons.git for the submodule

@mattab commented on February 21st 2017 Owner

FYI @Findus23 renamed the repository to piwik-icons

@Findus23 commented on February 21st 2017 Member

I have now changed the url in .gitmodules (I hope that's enough)
https://github.com/piwik/piwik/pull/11383/commits/75334558070afa2ef9b4b97de991c8db1f2e44aa

@mattab commented on February 21st 2017 Owner

Note: before merging we need to change the build script

wip in https://github.com/piwik/piwik-package/pull/52/files

@Findus23 commented on February 21st 2017 Member

SystemTests

https://travis-ci.org/piwik/piwik/jobs/203797459
Because of plugins/CustomDimensions/tests/System/expected/test___Live.getLastVisitsDetails_year.xml

IntegrationTests

https://travis-ci.org/piwik/piwik/jobs/203797465:
Maybe we should delete the irrelevant files before the IntegrationTests

1) Piwik\Tests\Integration\ReleaseCheckListTest::test_icoFilesIconsShouldBeInPngFormat

source favicons

2) Piwik\Tests\Integration\ReleaseCheckListTest::testTemplatesDontContainJquery with data set #1 ('html')

index.html in the flag-icon-css repo

3) Piwik\Tests\Integration\ReleaseCheckListTest::test_TotalPiwikFilesSize_isWithinReasonnableSize

UPDATE: they should be fixed in https://github.com/piwik/piwik/pull/11383/commits/978fc56390e500227dcd9ae3f33156c4968e8e6b

Unittests

https://travis-ci.org/piwik/piwik/jobs/203797466
1-3

Solved in 955b8382ac

Rest:

4) Piwik\Plugins\Referrers\tests\SearchEngineTest::testMissingSearchEngineIcons with data set #0 ('1.cz', array(array('/s\/([^\/]+)/', 'q'), 's/{k}', array('iso-8859-2'), '1.cz'))
1.cz
Failed asserting that false is true.

Not sure where the problem is.

UI-Tests

seem to fail because of the change in transparency and this errors (which shouldn't have to do with my change)

http://builds-artifacts.piwik.org/piwik/piwik/3.x-dev/22428/UIIntegrationTest_admin_plugins.png
http://builds-artifacts.piwik.org/piwik/piwik/3.x-dev/22428/Login_login_form.png

@Findus23 commented on March 3rd 2017 Member

I tried to merge the conflicts. we'll see which tests are failing now.

@Findus23 commented on March 3rd 2017 Member

There are some icons in piwik that are not in my repository (mostly because the sites are offline)
https://travis-ci.org/piwik/piwik/jobs/207478249

Only in piwik/plugins/Referrers/images/searchEngines: 1.cz.png
Only in piwik/plugins/Referrers/images/searchEngines: apollo.lv.png
Only in piwik/plugins/Referrers/images/searchEngines: arama.com.png
Only in piwik/plugins/Referrers/images/searchEngines: bg.setooz.com.png
Only in piwik/plugins/Referrers/images/searchEngines: blogsearch.google.com.png
Only in piwik/plugins/Referrers/images/searchEngines: busca.orange.es.png
Only in piwik/plugins/Referrers/images/searchEngines: chercherfr.aguea.com.png
Only in piwik/plugins/Referrers/images/searchEngines: claro-search.com.png
Only in piwik/plugins/Referrers/images/searchEngines: daemon-search.com.png
Only in piwik/plugins/Referrers/images/searchEngines: encrypted.google.com.png
Only in piwik/plugins/Referrers/images/searchEngines: eo.st.png
Only in piwik/plugins/Referrers/images/searchEngines: extern.peoplecheck.de.png
Only in piwik/plugins/Referrers/images/searchEngines: fr.dir.com.png
Only in piwik/plugins/Referrers/images/searchEngines: fr.wedoo.com.png
Only in piwik/plugins/Referrers/images/searchEngines: gais.cs.ccu.edu.tw.png
Only in piwik/plugins/Referrers/images/searchEngines: geona.net.png
Only in piwik/plugins/Referrers/images/searchEngines: googlesyndicatedsearch.com.png
Only in piwik/plugins/Referrers/images/searchEngines: holmes.ge.png
Only in piwik/plugins/Referrers/images/searchEngines: iwon.ask.com.png
Only in piwik/plugins/Referrers/images/searchEngines: junglekey.com.png
Only in piwik/plugins/Referrers/images/searchEngines: ko.search.need2find.com.png
Only in piwik/plugins/Referrers/images/searchEngines: kwzf.net.png
Only in piwik/plugins/Referrers/images/searchEngines: laban.vn.png
Only in piwik/plugins/Referrers/images/searchEngines: lo.st.png
Only in piwik/plugins/Referrers/images/searchEngines: online.no.png
Only in piwik/plugins/Referrers/images/searchEngines: p.zhongsou.com.png
Only in piwik/plugins/Referrers/images/searchEngines: pesquisa.clix.pt.png
Only in piwik/plugins/Referrers/images/searchEngines: pesquisa.sapo.pt.png
Only in piwik/plugins/Referrers/images/searchEngines: recherche.francite.com.png
Only in piwik/plugins/Referrers/images/searchEngines: s1.metacrawler.de.png
Only in piwik/plugins/Referrers/images/searchEngines: scour.com.png
Only in piwik/plugins/Referrers/images/searchEngines: search.auone.jp.png
Only in piwik/plugins/Referrers/images/searchEngines: search.bluewin.ch.png
Only in piwik/plugins/Referrers/images/searchEngines: search.comcast.net.png
Only in piwik/plugins/Referrers/images/searchEngines: search.freecause.com.png
Only in piwik/plugins/Referrers/images/searchEngines: search.lilo.org.png
Only in piwik/plugins/Referrers/images/searchEngines: search.nate.com.png
Only in piwik/plugins/Referrers/images/searchEngines: search.seesaa.jp.png
Only in piwik/plugins/Referrers/images/searchEngines: search.smartaddressbar.com.png
Only in piwik/plugins/Referrers/images/searchEngines: search.smartshopping.com.png
Only in piwik/plugins/Referrers/images/searchEngines: search.yippy.com.png
Only in piwik/plugins/Referrers/images/searchEngines: searchalot.com.png
Only in piwik/plugins/Referrers/images/searchEngines: sp-image.search.auone.jp.png
Only in piwik/plugins/Referrers/images/searchEngines: suche.info.png
Only in piwik/plugins/Referrers/images/searchEngines: video.google.com.png
Only in piwik/plugins/Referrers/images/searchEngines: video.so-net.ne.jp.png
Only in piwik/plugins/Referrers/images/searchEngines: videosearch.nifty.com.png
Only in piwik/plugins/Referrers/images/searchEngines: web.canoe.ca.png
Only in piwik/plugins/Referrers/images/searchEngines: websearch.cs.com.png
Only in piwik/plugins/Referrers/images/searchEngines: www.123people.com.png
Only in piwik/plugins/Referrers/images/searchEngines: www.acoon.de.png
Only in piwik/plugins/Referrers/images/searchEngines: www.blogdigger.com.png
Only in piwik/plugins/Referrers/images/searchEngines: www.blogpulse.com.png
Only in piwik/plugins/Referrers/images/searchEngines: www.cuil.com.png
Only in piwik/plugins/Referrers/images/searchEngines: www.eurip.com.png
Only in piwik/plugins/Referrers/images/searchEngines: www.firstsfind.com.png
Only in piwik/plugins/Referrers/images/searchEngines: www.fixsuche.de.png
Only in piwik/plugins/Referrers/images/searchEngines: www.gomeo.com.png
Only in piwik/plugins/Referrers/images/searchEngines: www.hooseek.com.png
Only in piwik/plugins/Referrers/images/searchEngines: www.latne.lv.png
Only in piwik/plugins/Referrers/images/searchEngines: www.looksmart.com.png
Only in piwik/plugins/Referrers/images/searchEngines: www.maailm.com.png
Only in piwik/plugins/Referrers/images/searchEngines: www.mister-wong.com.png
Only in piwik/plugins/Referrers/images/searchEngines: www.monstercrawler.com.png
Only in piwik/plugins/Referrers/images/searchEngines: www.mozbot.fr.png
Only in piwik/plugins/Referrers/images/searchEngines: www.qualigo.at.png
Only in piwik/plugins/Referrers/images/searchEngines: www.searchy.co.uk.png
Only in piwik/plugins/Referrers/images/searchEngines: www.suchmaschine.com.png
Only in piwik/plugins/Referrers/images/searchEngines: www.toile.com.png
Only in piwik/plugins/Referrers/images/searchEngines: www.toolbarhome.com.png
Only in piwik/plugins/Referrers/images/searchEngines: www.trouvez.com.png
Only in piwik/plugins/Referrers/images/searchEngines: www.trusted-search.com.png
Only in piwik/plugins/Referrers/images/searchEngines: www.vindex.nl.png
Only in piwik/plugins/Referrers/images/searchEngines: www.walhello.info.png
Only in piwik/plugins/Referrers/images/searchEngines: www.zxuso.com.png
Only in piwik/plugins/Referrers/images/searchEngines: www2.austronaut.at.png

I'll copy them over so the aren't missing for old sites.

@mattab commented on March 25th 2017 Owner

This looks great @Findus23

Review:

  • Left a couple comments
  • Could you also rebase the branch and get latest changes from 3.x-dev
  • (optional) fix the UI test by running the console command output at the top of the UI test build which will download all screenshots (i'll do it if you don't no worries - all UI changes look valid)

Then it looks good to be merged! :+1:

@Findus23 commented on March 25th 2017 Member

Left a couple comments

I only saw one and fixed it

Could you also rebase the branch and get latest changes from 3.x-dev

https://github.com/piwik/piwik/pull/11383/commits/99d913eff84f6200e4fdb05f432a62dd8f15c80a

(optional) fix the UI test by running the console command output at the top of the UI test build which will download all screenshots (i'll do it if you don't no worries - all UI changes look valid)

I'd prefer you to add them as I don't have git LFS installed.

@mattab commented on March 28th 2017 Owner

Re-opened PR here: #11548 so we can add commits to it. @Findus23 in the future maybe you could create the branch directly in the Piwik repo. We would be very happy to welcome you in the Piwik organisation! :1st_place_medal:

@mattab commented on March 28th 2017 Owner
This Pull Request was closed on March 28th 2017
Powered by GitHub Issue Mirror