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.

Sign up for GitHub

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

Jump to bottom

Fixed #31700 -- Made makemigrations command display meaningful symbols for each operation. #17344

Merged
merged 1 commit into from Jan 17, 2024
Merged

Fixed #31700 -- Made makemigrations command display meaningful symbols for each operation. #17344

merged 1 commit into from Jan 17, 2024

Conversation

AMK9978
Copy link
Contributor

@AMK9978 AMK9978 commented Oct 6, 2023 •
edited

The original pull request was #17273. Unfortunately, I removed my previous forked project.
Forum thread

felixxm
felixxm reviewed Nov 2, 2023
View reviewed changes
Copy link
Member

@felixxm felixxm left a comment

Choose a reason for hiding this comment

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

@AMK9978 Thanks 👍 I think the current thread is to use more symbols and stop using colors (\cc @django/accessibility). Also, if we're going to use symbols as pictograms we should document their meaning. Can you work on this?

django/core/management/commands/makemigrations.py Outdated Show resolved Hide resolved
@knyghty
Copy link
Member

knyghty commented Nov 2, 2023 •
edited

IMO it's fine (good, even) to add the colours as long as the symbols are enough on their own.

@AMK9978
Copy link
Contributor Author

AMK9978 commented Nov 2, 2023

@AMK9978 Thanks 👍 I think the current thread is to use more symbols and stop using colors (\cc @django/accessibility). Also, if we're going to use symbols as pictograms we should document their meaning. Can you work on this?

I think I need to write docs in the release/5.0.txt and also note the category of each operation.

felixxm
felixxm reviewed Nov 14, 2023
View reviewed changes
Copy link
Member

@felixxm felixxm left a comment

Choose a reason for hiding this comment

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

@AMK9978 Is there a reason why you left only two categories? (check out comment). Also, all examples of makemigrations outputs should be updated in docs.

docs/releases/5.0.txt Outdated Show resolved Hide resolved
django/contrib/postgres/operations.py Outdated Show resolved Hide resolved
docs/ref/migration-operations.txt Outdated Show resolved Hide resolved
felixxm
felixxm reviewed Nov 29, 2023
View reviewed changes
Copy link
Member

@felixxm felixxm left a comment

Choose a reason for hiding this comment

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

@AMK9978 Thanks 👍 I left move comments.

docs/ref/migration-operations.txt Show resolved Hide resolved
docs/ref/migration-operations.txt Outdated Show resolved Hide resolved
docs/ref/migration-operations.txt Outdated Show resolved Hide resolved
docs/ref/migration-operations.txt Outdated Show resolved Hide resolved
django/db/migrations/operations/base.py Outdated Show resolved Hide resolved
django/db/migrations/operations/base.py Outdated Show resolved Hide resolved
django/db/migrations/operations/base.py Outdated Show resolved Hide resolved
docs/releases/5.1.txt Show resolved Hide resolved
David-Wobrock
David-Wobrock approved these changes Dec 30, 2023
View reviewed changes
Copy link
Member

@David-Wobrock David-Wobrock left a comment

Choose a reason for hiding this comment

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

Neat change 🙆

Nothing to add on the code side 👍

felixxm
felixxm reviewed Jan 3, 2024
View reviewed changes
docs/releases/5.1.txt Outdated Show resolved Hide resolved
charettes
charettes reviewed Jan 4, 2024
View reviewed changes
django/db/migrations/operations/base.py Outdated Show resolved Hide resolved
django/db/migrations/operations/base.py Outdated Show resolved Hide resolved
django/db/migrations/operations/base.py Outdated Show resolved Hide resolved
@felixxm
Copy link
Member

felixxm commented Jan 17, 2024

@AMK9978 I'm working on this patch, please don't push more edits.

@felixxm
Copy link
Member

felixxm commented Jan 17, 2024

Added tests for django.contrib.postgres operations.

@felixxm felixxm changed the title Refs #31700 -- Added formatted description to highlight different operations in makemigrations output. Fixed #31700 -- Made makemigrations command display meaningful symbols for each operation. Jan 17, 2024
felixxm
felixxm reviewed Jan 17, 2024
View reviewed changes
Copy link
Member

@felixxm felixxm left a comment

Choose a reason for hiding this comment

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

@AMK9978 Thanks 👍

I pushed final edits to docs.

docs/ref/migration-operations.txt Outdated Show resolved Hide resolved
@felixxm felixxm merged commit 27a3eee into django:main Jan 17, 2024
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@charettes charettes charettes left review comments

@shangxiao shangxiao shangxiao left review comments

@felixxm felixxm felixxm left review comments

@knyghty knyghty knyghty left review comments

@David-Wobrock David-Wobrock David-Wobrock approved these changes

Assignees
No one assigned
Labels
None yet
Projects
django accessibility improvements
Status: Done
Milestone
No milestone
6 participants
@AMK9978 @knyghty @felixxm @charettes @shangxiao @David-Wobrock