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

Removed loading of deleted list-feature.js from base.html #1917

Merged
merged 1 commit into from
Jan 31, 2025

Conversation

bmispelon
Copy link
Member

list-feature.js was deleted in af5cd63

@bmispelon
Copy link
Member Author

@adamzap I believe this was the cause of the crash when deploying this (had nothing to do with out deploy script). I should have spotted that during review, sorry.

Copy link
Member

@pauloxnet pauloxnet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bmispelon bmispelon merged commit 6c5ce0a into django:main Jan 31, 2025
4 checks passed
@bmispelon bmispelon deleted the list-feature-delete branch January 31, 2025 07:42
@adamzap
Copy link
Member

adamzap commented Jan 31, 2025

@bmispelon Wow, I'm not sure how I missed this, sorry about that! I thought we had a test that makes sure collectstatic works in a production-like environment?

This has taught me that require.js fails silently and produced no console output on missing references. Very weird!

@bmispelon
Copy link
Member Author

I thought we had a test that makes sure collectstatic works in a production-like environment?

We have an issue to implement such a test, but it hasn't been done yet: #1831 (there's a PR for that ticket, but it's not in a mergeable state right now and I haven't had time to give feedback on it).

But that wouldn't have caught this issue anyway, since collectstatic would have worked. It might work if we run the test suite with a ManifestStorage for static files (assuming there's at least one test that ends up rendering the base.html).

In any case, that's a failure on the part of our test suite, not on yours. Plus all it did was cost us a few minutes of down time, and then we ended up making our deploy script more robust. Not a bad outcome at all 😁

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.

3 participants