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

Fix Markdown type:ignore #241

Merged
merged 2 commits into from
Apr 16, 2024
Merged

Conversation

llucax
Copy link
Contributor

@llucax llucax commented Apr 15, 2024

No description provided.

@llucax llucax requested a review from a team as a code owner April 15, 2024 15:28
@llucax llucax self-assigned this Apr 15, 2024
@github-actions github-actions bot added the part:template Affects the cookiecutter template files label Apr 15, 2024
@llucax
Copy link
Contributor Author

llucax commented Apr 15, 2024

Based on:

@llucax llucax requested a review from Marenz April 15, 2024 15:29
@llucax llucax added this to the v0.10.0 milestone Apr 15, 2024
-e '/# `slugify_unicode` function, but it definitely does./d' \
docs/_scripts/macros.py
echo
manual_step "Please make sure that the 'Markdown' and 'types-Markdown' dependencies are at version 3.5.2 or higher in 'pyproject.toml':"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why must this be a manual step?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it is too time consuming and complicated to make it automatic, I just wanted to strike a good balance to still have time to do other stuff that has more priority than this 🤷

Copy link
Contributor

@Marenz Marenz left a comment

Choose a reason for hiding this comment

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

Why the manual step? Sounds easy enough to automate? :)

@llucax
Copy link
Contributor Author

llucax commented Apr 16, 2024

Why the manual step? Sounds easy enough to automate? :)

It sounds quite time consuming to me, parsing python versions properly is tricky, and updating the pyproject.toml file without messing with the format and comments might be too (I don't know and it would take time to even find out).

@llucax
Copy link
Contributor Author

llucax commented Apr 16, 2024

For me it was a "get 90% of the benefits doing 10% of the work" kind of thing.

llucax added 2 commits April 16, 2024 09:06
The new version solves a typing issue we needed to ignore in the past.

Using the fixed version as the minimum avoid problems when upgrading the
`Markdown` in new projects.

Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
@llucax
Copy link
Contributor Author

llucax commented Apr 16, 2024

Rebased and enabled auto-merge.

@llucax llucax enabled auto-merge April 16, 2024 12:07
@llucax llucax added part:tooling Affects the development tooling (CI, deployment, dependency management, etc.) type:enhancement New feature or enhancement visitble to users labels Apr 16, 2024
@llucax llucax requested a review from Marenz April 16, 2024 12:08
Copy link
Contributor

@Marenz Marenz left a comment

Choose a reason for hiding this comment

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

Yeah okay, after playing a bit with the matter I notice it's not that trivial :D

@llucax llucax added this pull request to the merge queue Apr 16, 2024
Merged via the queue into frequenz-floss:v0.x.x with commit 7f74205 Apr 16, 2024
16 checks passed
@llucax llucax deleted the fix-type-ignore branch April 16, 2024 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
part:template Affects the cookiecutter template files part:tooling Affects the development tooling (CI, deployment, dependency management, etc.) type:enhancement New feature or enhancement visitble to users
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants