Skip to content

Pre-release improvements #28

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

Merged
merged 8 commits into from
Oct 6, 2023
Merged

Pre-release improvements #28

merged 8 commits into from
Oct 6, 2023

Conversation

blag
Copy link
Collaborator

@blag blag commented Sep 22, 2023

I'll be releasing a new version from master once this is merged.

Changes:

  • Use Python 3 syntax
  • Use modern Django features (the only change here is valid since Django 2.2)
  • Merge Django 4 branch
  • Update version to 0.10.0 in setup.py

@blag blag mentioned this pull request Sep 22, 2023
@blag blag requested review from andybak and fruitschen September 22, 2023 22:18
@blag
Copy link
Collaborator Author

blag commented Sep 23, 2023

I migrated everything to use pyproject.toml and Poetry to build and publish the package. Nothing against PyPA, but I find Poetry a joy to use compared to setuptools, and its packaging configuration a much more straightforward configuration. But if any reviewers would strongly prefer to switch back to setuptools (also using pyproject.toml), I am willing to do so.

Copy link

@amaddio amaddio left a comment

Choose a reason for hiding this comment

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

Django 4.2

  • ✅ url has been replaced with re_path
  • ✅ ugettext_lazy has been replaced with gettext_lazy

_request

Line 73 could raise an AttributeError if self is undefined. See: AttributeError

I'd either pass a default, wrap an try/except or add a docblock to the function body that an error could be raised.

html = Dropdown(
                label=_("Actions"),
                items=items,
                request=getattr(self, '_request')
            ).render()

_request is obviously defined in the order the functions are called. The order of the functions being called should not determine if the code works or not. A future developer might update another thing and change the order of the functions call, causing an error he/she does not understand.

Quotes

I would opt for either single or double quotes. I'd not mix it. Personally I prefer single ones for string constants. Doubles to escape.

"In Python, single-quoted strings and double-quoted strings are the same. This PEP does not make a recommendation for this. Pick a rule and stick to it."

source: https://peps.python.org/pep-0008/#string-quotes

@blag
Copy link
Collaborator Author

blag commented Oct 5, 2023

@amaddio Does this now look good to go to you?

@amaddio
Copy link

amaddio commented Oct 6, 2023

@blag Looking great to me.

@blag blag merged commit f3f44a8 into master Oct 6, 2023
@blag blag deleted the prerelease-improvements branch October 6, 2023 08:09
@blag
Copy link
Collaborator Author

blag commented Oct 6, 2023

Version 0.1.0 is now available on PyPI and GitHub.

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.

3 participants