Joomla! Issue Tracker - CMS

Download
Launch
Feeling Lucky

[#31210] - [4.0] Remove unused scss file

NPM Resource Changed ? Pending

User tests: Successful: Unsuccessful:

avatar Quy
Quy
22 Oct 2020

Testing Instructions

Code review.

avatar Quy Quy - open - 22 Oct 2020
avatar Quy Quy - change - 22 Oct 2020
Status New ⇒ Pending
avatar joomla-cms-bot joomla-cms-bot - change - 22 Oct 2020
Category ⇒ Administration Templates (admin) NPM Change
avatar brianteeman brianteeman - test_item - 22 Oct 2020 - Tested successfully
avatar brianteeman
brianteeman - comment - 22 Oct 2020

I have tested this item ✅ successfully on 5336a21

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31210.

avatar N6REJ N6REJ - test_item - 23 Oct 2020 - Tested successfully
avatar N6REJ
N6REJ - comment - 23 Oct 2020

I have tested this item ✅ successfully on 5336a21

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31210.

avatar SharkyKZ SharkyKZ - change - 23 Oct 2020
Status Pending ⇒ Ready to Commit
Labels Added: ? NPM Resource Changed
avatar SharkyKZ
SharkyKZ - comment - 23 Oct 2020

RTC.

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31210.

avatar SharkyKZ SharkyKZ - change - 23 Oct 2020
Status Ready to Commit ⇒ Pending
avatar SharkyKZ
SharkyKZ - comment - 23 Oct 2020

Sorry, setting back to pending. Tests are failing. The file is still referenced in administrator/templates/atum/scss/vendor/choicesjs/choices.scss

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31210.

avatar brianteeman
brianteeman - comment - 23 Oct 2020

Good spot. Annoyed I missed that

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31210.

avatar Quy Quy - change - 23 Oct 2020
Labels
avatar hans2103 hans2103 - test_item - 24 Oct 2020 - Tested successfully
avatar hans2103
hans2103 - comment - 24 Oct 2020

I have tested this item ✅ successfully on 9ab0066

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31210.

avatar richard67
richard67 - comment - 26 Oct 2020

@Quy Now drone says in the "npm" step:

Error: File to import not found or unreadable: ../../mixin.
        on line 7 of administrator/templates/atum/scss/vendor/joomla-custom-elements/joomla-tab.scss
>> @import "../../mixin";
avatar richard67
richard67 - comment - 26 Oct 2020

So as we can see, a code review is NOT sufficient to test such PRs. It needs to run npm to see if scss files are still compiled without errors, or at least to check drone.

And if code review, then a simple findstr /s /c:"../mixin" *.scss in a Windows command shell or find ./ -type f -name "*\.scss" -exec grep -Hn "../mixin" {} \; in a Linux shell would have shown the remaining imports, too.

Code review doesn't mean to look only on the changes in a PR, it means also e.g. if something is removed from code to make sure it is not used anywhere else.

avatar Quy Quy - change - 14 Jul 2021
Status Pending ⇒ Closed
Closed_Date 0000-00-00 00:00:00 ⇒ 2021-07-14 15:46:46
Closed_By ⇒ Quy
avatar Quy Quy - close - 14 Jul 2021

Add a Comment

Login with GitHub to post a comment