django-cms / djangocms-text-ckeditor

Text Plugin for django CMS using CKEditor 4
https://www.django-cms.org/en/repositories-plugins/
BSD 3-Clause "New" or "Revised" License
167 stars 185 forks source link

Fix pending migration implied by Django 4.0 changes (#634) #635

Closed sveetch closed 1 year ago

sveetch commented 1 year ago

I've launched tox on my work and it runned almost well but failed on environments with Django 3.2 because of missing migrations from some dependencies (filer, easy_thumbnails), possibly because of the same bug we are fixing here.

I'm attaching the full tox run output here so you can check yourself without to run it yet:

tox_log_output.txt

Also i'm not totally sure about my migration which have a dependency to the last CMS one, but i saw there is no CMS migration dependency in any of the djangocms-text-ckeditor migrations, maybe you were removing them for a reason, or this is normal, i don't know..

Let me know if you want me to remove it if it is not expected.

lgtm-com[bot] commented 1 year ago

This pull request introduces 3 alerts when merging 8c36817a3dd637c0d3370734117222bb3c726e46 into 3c34d2d5ab2a186389ee7fe5947024cea9b0b9bd - view on LGTM.com

new alerts:

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine :gear: that powers LGTM.com. For more information, please check out our post on the GitHub blog.

codecov[bot] commented 1 year ago

Codecov Report

Merging #635 (0490f9c) into master (3c34d2d) will decrease coverage by 5.05%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #635      +/-   ##
==========================================
- Coverage   74.91%   69.86%   -5.06%     
==========================================
  Files          20       16       -4     
  Lines         905      448     -457     
  Branches      128       49      -79     
==========================================
- Hits          678      313     -365     
+ Misses        199      118      -81     
+ Partials       28       17      -11     
Impacted Files Coverage Δ
...editor/migrations/0005_alter_text_cmsplugin_ptr.py 100.00% <100.00%> (ø)
djangocms_text_ckeditor/cms_plugins.py
djangocms_text_ckeditor/__init__.py
djangocms_text_ckeditor/settings.py
djangocms_text_ckeditor/forms.py
djangocms_text_ckeditor/models.py

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

marksweb commented 1 year ago

On a side note, coverage reporting needs to ignore migration files.

lgtm-com[bot] commented 1 year ago

This pull request introduces 3 alerts when merging 496b4a8327092375722623659f441cfaefbb1d67 into 3c34d2d5ab2a186389ee7fe5947024cea9b0b9bd - view on LGTM.com

new alerts:

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine :gear: that powers LGTM.com. For more information, please check out our post on the GitHub blog.

fsbraun commented 1 year ago

As a remark: Migration tests are fixed in #631