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

Modernization - May 2024 #16

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Modernization - May 2024 #16

wants to merge 9 commits into from

Conversation

dbaty
Copy link
Member

@dbaty dbaty commented May 15, 2024

Commits should be reviewed one by one.

@dbaty dbaty force-pushed the dbaty/modernization branch 3 times, most recently from 7e7630d to c0a9c4c Compare May 15, 2024 06:01
dbaty added 7 commits May 15, 2024 17:17
Active support of Python 3.7 has ended in June 2020. Security support
has ended in June 2023.
The "pyproject" extra dependency has been removed from isort 5.0.0,
which has been released in July 2020. It caused a warning with when a
more recent version was installed:

    $ pip install -r requirements_dev.txt
    [...]
    WARNING: isort 5.13.2 does not provide the extra 'pyproject'
    [...]
We maintained non-obvious code to support versions of Git that did not
have the `--only-matching` option, which appeared in Git 2.19. This
version was released in September 2018, it's time to move on.
That's where all tools are configured (except ReadTheDocs, which does
not support `pyproject.toml`).
These checks seem useful, I don't know why they should disabled.
@dbaty dbaty force-pushed the dbaty/modernization branch from f9bccf9 to bf5b75f Compare May 15, 2024 15:17
It was not necessary with Python >= 3.11, which ships with the
`tomllib` package in the standard library.

Also, the extra requirement could be a bit confusing, especially for
non-Python developers. "Do I need it now, will I need it later?"

For the sake of simplicity, the extra requirement has been removed.
We now always install `toml` on Python versions earlier than 3.11 (and
never on 3.11 and above).
@dbaty dbaty force-pushed the dbaty/modernization branch 2 times, most recently from 622bed5 to f40c3be Compare May 15, 2024 16:03

# Template used to display messages. This is a python new-style format string
# used to format the message information. See doc for all details
msg-template="{path}:{line}: [{msg_id}({symbol}), {obj}] {msg}"

Choose a reason for hiding this comment

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

There is no copy of the REPORTS section. Is this done on purpose?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this is intentional. I am switching to the default template for simplicity's sake. I find it readable enough: {path}:{line}:{column}: {msg_id}: {msg} ({symbol}). Example:

src/check_oldies/commands.py:33:4: W0612: Unused variable 'a' (unused-variable)

@@ -0,0 +1,2 @@
# TIMEBOMB: FEWTURE-BOOM1
# FEWTURE-DO-NOT-REPORT: do not report, it's not within an annotation

Choose a reason for hiding this comment

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

J'ai bien envie de d'utiliser le terme FEWTURE premier degré maintenant 😄

future_tag_regex=base.TESTING_FUTURE_TAG,
whitelist=(),
)
assert tags == {"FEWTURE-BOOM1", "FEWTURE-BOOM2"}

Choose a reason for hiding this comment

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

Est-ce que le tag n'est pas TIMEBOMB ici ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Le tag cherché est bien de type "FUTURE-xxx".

"TIMEBOMB", c'est en fait un "FIXME". Peut-être que je devrais le nommer "FIIXME" dans les tests, ce sera plus proche de "FIXME" et donc plus clair.

@dbaty dbaty force-pushed the dbaty/modernization branch from 2877df3 to f792408 Compare May 22, 2024 07:04
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