Conversation
|
Just for giggles, my first run of vendor/bundle/ is still 1.3 GB though. |
zachmargolis
left a comment
zachmargolis
left a comment
There was a problem hiding this comment.
LGTM small formatting comments
Makefile
Outdated
| download_acuant_sdk: ## Downloads the most recent Acuant SDK release from Github | ||
| @scripts/download_acuant_sdk.sh | ||
|
|
||
| clobber_db: ## resets the database for make setup |
There was a problem hiding this comment.
extra space
| clobber_db: ## resets the database for make setup | |
| clobber_db: ## resets the database for make setup |
There was a problem hiding this comment.
👍
I think this explanation makes it worse, but: I had done this deliberately, because it looked like that was the pattern used elsewhere in the file. But looking at it again, nothing else uses 2 spaces. Maybe I saw the two pound signs and wires got crossed in my mind...? Will fix these.
Makefile
Outdated
| bin/rake db:environment:set | ||
| bin/rake dev:prime | ||
|
|
||
| clobber_assets: ## removes assets |
There was a problem hiding this comment.
extra space
| clobber_assets: ## removes assets | |
| clobber_assets: ## removes assets |
Makefile
Outdated
| bin/rake assets:clobber | ||
| RAILS_ENV=test bin/rake assets:clobber | ||
|
|
||
| clobber_logs: ## purges logs |
There was a problem hiding this comment.
extra space ```suggestion
clobber_logs: ## purges logs
Makefile
Outdated
| ## Remove assets and logs, and unused gems, but leave DB alone | ||
| tidy: clobber_assets clobber_logs |
There was a problem hiding this comment.
putting the comment at the end lets make help read it correctly
| ## Remove assets and logs, and unused gems, but leave DB alone | |
| tidy: clobber_assets clobber_logs | |
| tidy: clobber_assets clobber_logs ## Remove assets and logs, and unused gems, but leave DB alone |
There was a problem hiding this comment.
On the "It would look better to say nothing" front... Until I saw this comment, I was unaware of make help! I thought we just had a pattern of commenting everything. 😇
Thanks for these comments; I'm going to clean these up and also make the comments read slightly better now that I have this context.
[skip changelog]
| clobber_assets: ## Removes (clobbers) assets | ||
| bin/rake assets:clobber | ||
| RAILS_ENV=test bin/rake assets:clobber |
There was a problem hiding this comment.
I'd personally find some value in this one, curious if it is aware of all of the asset outputs, i.e. public/assets (Sprockets), public/packs (Webpack/jsbundling-rails), app/assets/builds (Sass/cssbundling-rails).
There was a problem hiding this comment.
https://www.rubydoc.info/gems/sprockets-rails/2.3.3 suggests it isn't natively aware of some of those. public/packs is still there (15MB), for example. It looks like we can add them in an initializer, but I'd like to propose that be a separate PR if warranted.
There was a problem hiding this comment.
Yeah, to be fair, that seems like something the jsbundling-rails gem should be handling, since it's already doing things with other Rake tasks like rake assets:precompile.
There was a problem hiding this comment.
@n1zyy Based on what I see here, it can be achieved with javascript:clobber task:
There was a problem hiding this comment.
@aduth I like the idea, but I can't seem to verify what it's actually removing.
https://github.com/rails/jsbundling-rails/blob/main/lib/tasks/jsbundling/clobber.rake suggests that it may automatically extend assets:clobber.
There was a problem hiding this comment.
(More concretely: I can see that it runs rm_rf Dir["app/assets/builds/**/[^.]*.{js,js.map}"], verbose: false, but I don't actually have those files when running the app locally and don't follow why.)
There was a problem hiding this comment.
Oh! I forgot that we do things a little custom such that we don't output JavaScript to app/assets/builds, which probably explains why you're not seeing them and not seeing the task have any meaningful impact. We'd probably also need something custom to delete public/packs, then.
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
3 participants
Do people like this? This is a little bit of a ragecoded thing.
I realized my identity-idp clone was using over 4GB of disk space. The bulk of the ridiculous stuff was:
I wanted something like
make tidyto exist, but it was somewhat duplicative of whatmake setupdoes. I don't like runningmake setupoften because it's kind of heavy-handed and I don't want to mess with my DB. So I moved the relevant parts into individual Makefile targets.This is purely a "I want this to exist" thing, so if it's contentious I'm fine withdrawing it.