Uh oh!
There was an error while loading. Please reload this page.
Use underscores instead of spaces in paths - #332
Conversation
ezio-melotti
left a comment
There was a problem hiding this comment.
If I'm reading the code correctly, this value is retrieved here:
Lines 155 to 157 in a346755
and then used to create the filename in:
Lines 11 to 15 in a346755
There is only a test for this function:
Lines 42 to 46 in a346755
The changes look OK to me, even though they don't seem to be covered by the tests. If you think it's worth it, you could add another test for get_misc_news_filename, and possibly update its code to reject sections that include spaces.
hugovk
commented
Apr 3, 2023
Sure, updated! |
hugovk
commented
Jul 26, 2023
Updated to use underscores instead of hyphens, to match blurb, as decided during the EuroPython sprint: python/core-workflow#499 (comment) |
Is something still blocking this PR? |
hugovk
commented
Oct 12, 2023
Yes, python/core-workflow#499 needs merging first. |
ezio-melotti
commented
Oct 12, 2023
Is something blocking python/core-workflow#499? 🙃 |
hugovk
commented
Oct 12, 2023
Someone to hit the big green merge button? I think it's ready. |
Uh oh!
There was an error while loading. Please reload this page.
hugovk
commented
Jul 7, 2024
I've released blurb 1.2.0 that now uses underscores for the directories: https://github.com/python/blurb/releases/tag/v1.2.0 Let's give it a little bit of time for testing first, then we can mark this as ready for review/merge. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Éric <merwok@netwok.org>
Co-authored-by: Éric <merwok@netwok.org>
hugovk
commented
Jul 19, 2024
This is now ready for review/merge. After it's merged and we've confirmed it's working, I'll post on Discuss to encourage people to upgrade their blurb CLI. Further down the line, we can add a pre-commit check to prevent creation of dirs with spaces. |
hugovk
commented
Jul 25, 2024
We already have an approval, let's merge! 🚀 |
hugovk
commented
Jul 25, 2024
Confirmed working: python/cpython@ |
For python/core-workflow#186.
Companion to python/core-workflow#499.
I've not tested this other than running the unit tests.
I think this is all we need for blurb-it? For example, there's already "Tools-Demos" with no "/".