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

Creates lessc Makefile function to DRY up Less commands #1880

Conversation

shaneriley
Copy link
Contributor

Description: What does this PR achieve? [feature|hotfix|refactor]

Closes #1879

Technical: What should be noted about the implementation?

Refactoring to a function eliminated the default terminal output. Added an echo to note which file was being processed.

Testing: Steps for reviewer to reproduce / verify this PR fixes the problem?

Run docker-compose exec web make css

Evidence: If this PR touches UI, please post evidence (screenshot) of it behaving correctly:

N/A

@brad2014
Copy link

brad2014 commented Feb 14, 2019

If you wanted to do the old-fashioned make dependency approach, it might look something like this...

CSS_PAGES  = admin book edit form home lists plain subject user book-widget design dev 

.PHONY: css 
css: $(patsubst %,$(BUILD)/page-%.css,$(CSS_PAGES)) $(BUILD)/js-all.css

$(BUILD)/%.css : static/css/%.less $(BUILD)
  @echo Compressing $<
  lessc -x $< $@

$(BUILD) :
  mkdir -p $@

@jdlrobson
Copy link
Collaborator

This is cool, but wondering what it would take to move these into package.json ?

@shaneriley
Copy link
Contributor Author

We can go the watcher route instead, which would be great considering you don't have to run make every time you change styles, but it will require some rewriting and reorganization of the Less files. Adding https://www.npmjs.com/package/less-watch-compiler, for example, can allow us to specify the source and destination directories, but it will blindly compile each file individually which will lead to errors. There are a lot of Less files that assume previous Less files have been compiled in before it, and use variables that aren't instantiated in that file.

I'm not sure if we can have a similar setup with existing Less watchers, but when I use Sass I set up Chokidar to ignore files prepended with an underscore to get around that. I'll do some research and see if there's a similar option.

@jdlrobson
Copy link
Collaborator

I'm hoping to move this into webpack and drop the lessc compiler altogether.

See #1936 and #1123
I've also setup a board to track work for this project of JS modernization.
https://github.com/internetarchive/openlibrary/projects/10?add_cards_query=is%3Aopen

@mekarpeles
Copy link
Member

It looks like with #2003 this can be closed

@mekarpeles mekarpeles closed this Apr 9, 2019
@shaneriley
Copy link
Contributor Author

It looks like with #2003 this can be closed

👍 on it being irrelevant once #2003 is merged.

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.

Refactor CSS step in makefile
4 participants