-
Notifications
You must be signed in to change notification settings - Fork 402
Sort migrations by area and improve styling #1743
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like the effect. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Site works still, and any major concerns I had regarding how the Migration Guides generator would work from purely code review have been thoroughly debunked after testing this, so from a purely functional standpoint I feel comfortable with this PR.
However, I still have changes to request for consideration regarding clear documentation of what has changed in the Migration Guides generator.
(Also, does the migration guide generator still need to be run for the 0.15 guides? Those ones have Without Area at the top instead of bottom from what I can see).
And, finally, purely an observation, nitpick, and bikeshed that'd be best suited for a potential follow-up; perhaps we should more blatantly style the scope of each area? Like, some sort of border containing them, or lighter background behind them? Something to make it clear where one area ends and another begins.
Yes, the command now always overwrites the file because if we pick a new migration guide we don't were it's going to be located (previously it appended); assuming the guides list is not sorted, we must run the command. But the operation is non-destructive as it'll read the file before and only add the entries that are missing.
hahaha, ok, I'm not sure if I'll like wrapping the whole area section in some border or background… but I'm open to be convinced otherwise; maybe we can make the section header sticky so it's always present while scrolling? 🤔 |
Co-authored-by: TrialDragon <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm now.
|
Once CI is green I'll merge this @doup :) |
|
Ooops… @alice-i-cecile now is fixed. |
migration_guides.rsso that it generates sorted_guides.tomleach timebevy-migration-guides.mp4