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

remove no longer needed django-ckeditor package #1744

Merged
merged 2 commits into from
Jan 6, 2025

Conversation

goapunk
Copy link
Contributor

@goapunk goapunk commented Jan 6, 2025

Tasks

  • PR name contains story or task reference
  • Documentation (docs and inline)
  • Tests (including n+1 and django_assert_num_queries where applicable)
  • Changelog

@goapunk goapunk requested review from m4ra and hom3mad3 January 6, 2025 13:42
Copy link
Contributor

@m4ra m4ra left a comment

Choose a reason for hiding this comment

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

Yay

@m4ra
Copy link
Contributor

m4ra commented Jan 6, 2025

@goapunk some imports of ckeditor in templatetags need removal for tests to pass.

Copy link
Contributor

@m4ra m4ra left a comment

Choose a reason for hiding this comment

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

tests failing

@goapunk goapunk force-pushed the jd-2025-01-remove-django-ckeditor branch from ad74791 to 2fd23d0 Compare January 6, 2025 14:11
@goapunk goapunk requested a review from m4ra January 6, 2025 14:11
@goapunk
Copy link
Contributor Author

goapunk commented Jan 6, 2025

@m4ra fixed the test migrations now

import django_ckeditor_5.fields


class Migration(migrations.Migration):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@m4ra tbh I'm not entirely sure why it created this one just now

Copy link
Contributor

Choose a reason for hiding this comment

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

Because the 0006 has textfield, while the idea model has ckeditor 5. You don't need to change the 0006.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, good catch, I totally missed that it's already ckeditor5 🤯

import django_ckeditor_5.fields


class Migration(migrations.Migration):
Copy link
Contributor

Choose a reason for hiding this comment

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

Because the 0006 has textfield, while the idea model has ckeditor 5. You don't need to change the 0006.

@goapunk goapunk force-pushed the jd-2025-01-remove-django-ckeditor branch from 2fd23d0 to a7e5f91 Compare January 6, 2025 15:43
@goapunk goapunk requested a review from m4ra January 6, 2025 15:44
Copy link
Contributor

@m4ra m4ra left a comment

Choose a reason for hiding this comment

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

Thanks!

@m4ra m4ra merged commit ba214d1 into main Jan 6, 2025
5 checks passed
@m4ra m4ra deleted the jd-2025-01-remove-django-ckeditor branch January 6, 2025 15:49
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.

2 participants