-
Notifications
You must be signed in to change notification settings - Fork 3
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
fix: ModuleNotFoundError when CMS is not installed #56
base: main
Are you sure you want to change the base?
Conversation
Reviewer's Guide by SourceryThis pull request fixes a ModuleNotFoundError that occurs when django CMS is not installed. The fix involves adding a check to ensure that the 'cms' app is in the INSTALLED_APPS setting before attempting to import or use any CMS-related functionality. Additionally, type hints were added to improve code readability and maintainability. Class diagram showing TextConfig and related functions with type hintsclassDiagram
class TextConfig {
+str name
+str verbose_name
+str default_auto_field
+dict[str, str] inline_models
+ready()
}
class Functions {
+discover_inline_editable_models() dict[str, str]
+check_ckeditor_settings(app_configs, kwargs) list
+check_no_cms_config(app_configs, kwargs) list
}
note for TextConfig "Added type hint for inline_models"
note for Functions "Added return type hints to functions"
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @fsbraun - I've reviewed your changes - here's some feedback:
Overall Comments:
- Could you please explain in the PR description how these type annotation changes fix the ModuleNotFoundError mentioned in the title? This would help document the connection between the changes and issue Import error when using in projects without django CMS #55.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #56 +/- ##
==========================================
- Coverage 81.28% 80.89% -0.40%
==========================================
Files 17 17
Lines 935 942 +7
Branches 104 104
==========================================
+ Hits 760 762 +2
- Misses 132 137 +5
Partials 43 43 ☔ View full report in Codecov by Sentry. |
Fixes #55
Summary by Sourcery
Bug Fixes: