administrate
administrate copied to clipboard
Run diff-check on JS/CSS to catch updates
In #2673, we discussed catching changes in the builds directory as being quite annoying in development. This can happen when people contribute changes (and should probably also commit their build changes to make future development easier), plus also when dependencies change.
Catching this with diff-check should mean that we see this at the point of introduction, rather than later on.
I do feel like it's not very clear what you should do with that failure, maybe it needs a custom message or something to make it clearer.
Is there any reason not to just add the build files to .gitignore as suggested by @goosys https://github.com/thoughtbot/administrate/pull/2673#issuecomment-2394008339
Just to confirm: this is intended to force contributors to check in any changes to compiled assets, right? In which case, it gets my vote, unless I'm missing a use case somewhere :slightly_smiling_face:
@littleforest - The issue I'm seeing is that any assets missing from the repo are not available when pulling the gem from Git (eg: gem "administrate", git: ....
this is intended to force contributors to check in any changes to compiled assets
Yeah, that's my thinking. If you make changes, your build will fail and prompt you to run the build.
A little while after I did this, it occurred to me that it solves another problem quite nicely, too: it stops us from having to make sure the prerelease Administrate demo is compiled correctly.
On this though, next I'd like to make sure it's got a better suggestion of what to do. Then once we've cut a new release we can merge this in.
I figured out why this kept failing, but I couldn't locally and just merged in #2710. It was a tiny change by JS bundling standards, and caused by how npm actually works (badly…).
Anyway, the solution is to do something like this:
git clean -fdx
bin/setup
yarn install
yarn build
yarn build:css
diff-check doesn't actually have a way to prompt what to do yet. I'd like to add that, but I'd also like to merge this sooner than later, so I'm going to do that.