Run diff-check on JS/CSS to catch updates #2680
Conversation
|
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 |
|
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 🙂 @littleforest - The issue I'm seeing is that any assets missing from the repo are not available when pulling the gem from Git (eg: |
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. |
nickcharlton
force-pushed
the
nc-run-diff-check-over-builds
branch
2 times, most recently
from
03dbbd8 to
afabe04
Compare
nickcharlton
force-pushed
the
nc-run-diff-check-over-builds
branch
from
afabe04 to
e28bb92
Compare
nickcharlton
force-pushed
the
nc-run-diff-check-over-builds
branch
from
e28bb92 to
4234c58
Compare
`npm`/`yarn` is non-deterministic when it comes to dependencies, and so you can very easily end up with a dependency drift (especially when you're being strict about what you build). It turns out that this is what #2680 was showing. If we clean the whole environment (`git clean -fdx`), then run `setup` and the other steps that the Action runs through, we get the same result.
nickcharlton
force-pushed
the
nc-run-diff-check-over-builds
branch
from
a911854 to
94443c2
Compare
`npm`/`yarn` is non-deterministic when it comes to bundling dependencies, and so you can very easily end up with a dependency drift (especially when you're being strict about what you build elsewhere). It turns out that this is what #2680 was showing. If we clean the whole environment (`git clean -fdx`), then run `setup` and the other steps that the Action runs through, we get the same result.
`npm`/`yarn` is non-deterministic when it comes to bundling dependencies, and so you can very easily end up with a dependency drift (especially when you're being strict about what you build elsewhere). It turns out that this is what #2680 was showing. If we clean the whole environment (`git clean -fdx`), then run `setup` and the other steps that the Action runs through, we get the same result.
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.
nickcharlton
force-pushed
the
nc-run-diff-check-over-builds
branch
from
94443c2 to
a101982
Compare
|
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 Anyway, the solution is to do something like this: git clean -fdx
bin/setup
yarn install
yarn build
yarn build:css
|
In #2673, we discussed catching changes in the
buildsdirectory 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-checkshould mean that we see this at the point of introduction, rather than later on.