Skip to content

Comments

Remove legacy less processing#23181

Merged
KevinMind merged 1 commit intomasterfrom
kevinmind/addons/15460
Mar 21, 2025
Merged

Remove legacy less processing#23181
KevinMind merged 1 commit intomasterfrom
kevinmind/addons/15460

Conversation

@KevinMind
Copy link
Contributor

@KevinMind KevinMind commented Mar 17, 2025

Fixes: mozilla/addons#15460

Context

We should merge this last in the chain of related PRs to remove legacy css processing as all assets will then be processed by vite.

Testing

Essentially should be able to run make up in prod or dev

make up DOCKER_TARGET=<development|production>

That is covered by CI

Checklist

  • Add #ISSUENUM at the top of your PR to an existing open issue in the mozilla/addons repository.
  • Successfully verified the change locally.
  • The change is covered by automated tests, or otherwise indicated why doing so is unnecessary/impossible.
  • Add before and after screenshots (Only for changes that impact the UI).
  • Add or update relevant docs reflecting the changes made.

@KevinMind KevinMind force-pushed the kevinmind/addons/15460 branch 6 times, most recently from a9f5858 to bd5b211 Compare March 20, 2025 17:26
@KevinMind KevinMind requested review from a team, diox and eviljeff and removed request for a team and diox March 20, 2025 17:27
@eviljeff
Copy link
Member

Does this need some rebasing? The title is "Remove legacy less processing" but there's a bunch of javascript changes too.

@KevinMind KevinMind force-pushed the kevinmind/addons/15460 branch from bd5b211 to 9e90bca Compare March 20, 2025 17:41
@KevinMind
Copy link
Contributor Author

KevinMind commented Mar 20, 2025

Does this need some rebasing? The title is "Remove legacy less processing" but there's a bunch of javascript changes too.

Those changes are the result of using eslint on the code. We now actually enforce linting rules and so we can autofix what hasn't been caught before. 3 commits:

  • remove legacy processing
  • add proper linting via eslint/prettier/knip
  • fix the errors that come from step 2

I could split this into yet another pair of PRs but at this point we are approaching a chain reaction where the number of PRs will soon be greater than the number of lines changed.. I kind of want to be done with this 🙃 😩

@eviljeff
Copy link
Member

I could split this into yet another pair of PRs but at this point we are approaching a chain reaction where the number of PRs will soon be greater than the number of lines changed.. I kind of want to be done with this 🙃 😩

I'm not saying the PR is unreviewable (though I haven’t tried yet) just that the PR title (and no description) implies a different scope than the contents of the PR.

@KevinMind KevinMind force-pushed the kevinmind/addons/15460 branch 3 times, most recently from 95af7be to d2d5c30 Compare March 20, 2025 18:45
@KevinMind
Copy link
Contributor Author

I could split this into yet another pair of PRs but at this point we are approaching a chain reaction where the number of PRs will soon be greater than the number of lines changed.. I kind of want to be done with this 🙃 😩

I'm not saying the PR is unreviewable (though I haven’t tried yet) just that the PR title (and no description) implies a different scope than the contents of the PR.

Split it.

@KevinMind KevinMind force-pushed the kevinmind/addons/15460 branch from d2d5c30 to 0a8cb3f Compare March 20, 2025 18:56
@KevinMind KevinMind force-pushed the kevinmind/addons/15460 branch from 0a8cb3f to 97350d2 Compare March 21, 2025 10:52
@KevinMind
Copy link
Contributor Author

KevinMind commented Mar 21, 2025

For posterity: reduced number of files by 7k.

before

  • Build
#28 17.61 8344 static files copied to '/data/olympia/site-static'.
#28 DONE 18.0s
  • Upload
succeeded 7 minutes ago in 2m 52s 

after

  • Build
#28 13.27 1006 static files copied to '/data/olympia/site-static'.
#28 DONE 13.7s
  • Upload
 succeeded 13 hours ago in 5m 11s 

@KevinMind KevinMind merged commit c88732e into master Mar 21, 2025
41 checks passed
@KevinMind KevinMind deleted the kevinmind/addons/15460 branch March 21, 2025 11:05
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.

[Task]: Remove legacy compress_assets and related libraries

2 participants