Or did you not push the update to gerrit? If not, I will do so.
Thanks. I will try that next time. Unfortunately, this did not fix the errors in CI. Are you still getting errors testing locally? I am not getting any errors locally.
I created a pull request for both Bootstrap and Chameleon to convert to using MediaWiki test classes and bootstrapping: https://github.com/ProfessionalWiki/Bootstrap/pull/36 and https://github.com/ProfessionalWiki/chameleon/pull/163. The tests pass on travis. It would be good to see if https://gerrit.wikimedia.org/r/c/mediawiki/skins/chameleon/+/597812 would pass CI with the changes.
Fri, May 29
Here is more detail on the approach we discussed:
Thu, May 28
OK, thanks for the context.
Tue, May 26
I removed the line causing the 'checkForUnintentionallyCoveredCode', and the warning went away, but everything else stayed the same.
This is what I'm getting testing locally:
@Krinkle, the code to be reviewed is listed above in the task description in the Which code to review section (the Chameleon skin, the Bootstrap extension, and the SCSS library). All of the code is currently hosted on github, although there is a task for figure out the best way to mirror the code from github to gerrit. Note that this task does not refer to WikimediaApiPortal, which is not yet ready for review. A separate task will be filed for that at the appropriate time.
There is a notable difference between running phpunit locally and in Wikimedia CI. https://github.com/ProfessionalWiki/chameleon/blob/master/docs/testing.md require adding -c phpunit.xml.dist, which may have inconsistent options for phpunit. Removing that run the local command results in multiple instances of the error:
So, it sounds like we're converging on:
I have have read and signed the L3 Wikimedia Server Access Responsibilities document.
Mon, May 25
If you create it, I'll merge it :-)
Ah, thanks for catching that! (My tests were with MW_INSTALL_PATH set, since my docker environment has skins/extensions in an alternative location.) Do you have a pull request for that, or shall I create one?
Thank you so much for the analysis and advice, @Daimona!
Pull request that adds phan and fixes all but one error: https://github.com/ProfessionalWiki/chameleon/pull/156. The remaining error is:
Sun, May 24
composer require --dev "mediawiki/mediawiki-phan-config: 0.10.2"
@Daimona, thank you! I was following the instructions at https://www.mediawiki.org/wiki/Continuous_integration/Tutorials/Add_phan_to_a_MediaWiki_extension. When I switched to your instructions, it worked!
Sat, May 23
I do indeed have the correct contents in .phan/config and have confirmed that https://github.com/wikimedia/mediawiki-tools-phan/blob/edf21edb0efdff1a2938842516fd0740df9fd240/src/config.php#L371 is executed. The assertion that is failing is
Fri, May 22
I strongly support this RFC. It will serve not only to document best practices for the use of composer for MediaWiki extensions and skins, but will also replace T467 with a policy that better reflects the current recommended state of the practice. I'll repeat my comment from T249573 below for the record.
When I try to run phan locally, I get:
What was the result of the research?
OK, thanks. Moving it to tech planning for review.
Thu, May 21
It looks like the github repo also is lowercase. See https://github.com/ProfessionalWiki/chameleon/issues/149, in which the fix went to lowercase. Would having a different casing have other implications, including issues with keeping the two repos synced? @JeroenDeDauw, thoughts?
Moving on workboard from Backlog to Next, since the Backlog column was hidden some time ago. Was T218207#5866709 ever resolved?
Moving on workboard from Backlog to Next, since the Backlog column was hidden some time ago. Is there work remaining for Core Platform Team to do on this task?
Moving on workboard from Backlog to Next, since the Backlog column was hidden some time ago. Is there work remaining to do on this task?
Thank you! That's great progress!
Is there anything for Core Platform Team to do on this task at this point?
Thanks for the clarification.
Sorry for the confusion. We do want it to be connected to SUL, but we want to configure access as specified in T249834.
Wed, May 20
It looks like this is a request for code review from Core Platform Team? Assigning to Core Platform Team Workboards (External Code Reviews), but please respond if the request is for something else. In the future, please tag Core Platform Team (instead of Core Platform Team Workboards (Clinic Duty Team)) and comment with the nature of the request. Thanks!
Tue, May 19
Mon, May 18
Seeking guidance from Release-Engineering-Team on the best approach for this. Thanks!