Skip to content
This repository has been archived by the owner on Dec 20, 2023. It is now read-only.

[NEW # 4949001] Ported latest articles cms plugin as package #1

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

sam-b0t
Copy link
Collaborator

@sam-b0t sam-b0t commented Oct 11, 2023

@sveetch Could you provide a list of essential/nice to have improvements on following subjects:

  • Sphinx documentation content:
    • Should I add some topics in it ? Clarify which subject should be expanded.
    • Is it configured correctly ?
  • Is setup.cfg correct ?
  • Is something missing in Makefile ?
  • Are sandbox settings correct ?

If you have any other suggestion, please advise.
NB: You might differentiate what is critical, from TODO.

…kage

[NEW] Add CI/CD workflows from firm_info and update with django and cms in test with tox

[NEW] Add issues templates from cms_plugin_blocks

[NEW] Add sphinx doc and config:

[CHG] Update tests to pass tox

AssertQuerySetEqual and AssertQuerysetEqual were not adequate for older/newer versions

Update codeQL workflow to run on pull_request on dev branch
@sam-b0t
Copy link
Collaborator Author

sam-b0t commented Oct 14, 2023

I'll update this portion of setup.cfg on monday: [options.package_data]

Copy link
Member

@sveetch sveetch left a comment

Choose a reason for hiding this comment

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

Your package does not follow the CMS plugin naming convention, package should be named cmsplugin-lotus and its main Python module should be cmsplugin_lotus

- Plateform: Window/IOS/Linux distribution
- Python version:
- Django version:

Copy link
Member

Choose a reason for hiding this comment

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

Add lines for "Lotus version" and "Lotus CMS plugin version"

.readthedocs.yml Outdated
# Optionally set the version of Python and requirements required to build your docs
python:
install:
- requirements: docs/requirements.txt
Copy link
Member

Choose a reason for hiding this comment

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

Usage of docs requirements is deprecated because it blocks you to test doc building on RTD on a non release version (the requirements file would want to install a packaged release only but RTD is able to build from any branch independently of package).

We prefer this way https://github.com/sveetch/py-css-styleguide/blob/master/.readthedocs.yml#L20 that only relate to "checkouted" branch, directly using the setup extra requirements.

MANIFEST.in Outdated
include LICENCE.txt
include README.rst

exclude sphinx_reload.py
Copy link
Member

Choose a reason for hiding this comment

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

This is not needed anymore

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed 5 first lines in fb3e044

@echo
@echo " freeze-dependencies -- to write installed dependencies versions in frozen.txt"
@echo " install -- to install this project with virtualenv and Pip"
@echo
Copy link
Member

Choose a reason for hiding this comment

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

Your makefile seems coming from Python package cookiecutter which does not include any Django related stuff like tasks migrate, check-migration, check-django, migrations, po, mo, etc..

Dependencies
************

* `Python`_>=3.8;
Copy link
Member

Choose a reason for hiding this comment

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

Missing dependencies for Django and DjangoCMS since there are essential dependencies that are important to announce.

"treebeard",
"menus",
"sekizai",
"lotus",
Copy link
Member

Choose a reason for hiding this comment

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

Lotus apps and settings should be in its own part below to avoid confusion with the DjangoCMS requirements.

(TEST_PAGE_TEMPLATES, "test-basic"),
)

BLOCKS_LOTUS_PLUGIN_TEMPLATES = [
Copy link
Member

Choose a reason for hiding this comment

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

This is not a block (from cmsplugin-blocks), this is lotus plugin, so LOTUS_CMSPLUGIN_TEMPLATES

install_requires =
django-cms>=3.11.0
django-blog-lotus[breadcrumbs]==0.7.0
django-autocomplete-light>=3.9.7
Copy link
Member

Choose a reason for hiding this comment

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

This is already defined from Lotus setup, don't enforce anything, this is Lotus that should manage this. Also you may relax lotus version requirement else you will have to make a new dummy release each time a new Lotus is released.


[tox:tox]
minversion = 3.4.0
envlist = py{38,310}-django{32,40,41,42}-cms{311}
Copy link
Member

Choose a reason for hiding this comment

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

Don't test every case, only the bounds else the tox run will be very long to finish. Bound versions are sufficient enough except for very exotic issue that are not worth to slow down quality process.

sphinx_reload.py Show resolved Hide resolved
@sam-b0t sam-b0t requested a review from sveetch October 16, 2023 05:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants