Skip to content

Fix issue (?) found by codespell#266

Merged
effigies merged 2 commits intopython-versioneer:masterfrom
DimitriPapadopoulos:typo
Sep 27, 2021
Merged

Fix issue (?) found by codespell#266
effigies merged 2 commits intopython-versioneer:masterfrom
DimitriPapadopoulos:typo

Conversation

@DimitriPapadopoulos
Copy link
Copy Markdown
Contributor

codespell complains:

./NOTES:60: delink ==> unlink

@effigies
Copy link
Copy Markdown
Contributor

Reading this, I believe it means that the file should still exist, but shouldn't be a hardlink or symlink to another file. I think "unlink" was specifically avoided for this reason.

@DimitriPapadopoulos
Copy link
Copy Markdown
Contributor Author

DimitriPapadopoulos commented Sep 27, 2021

I see. Should this perhaps be modified to something like the following?
make sure _version.py is not a symlink or hardlink before changing it

@effigies
Copy link
Copy Markdown
Contributor

Would codespell accept quotes around it, like "delink"? Or what about "break the link"?

@DimitriPapadopoulos
Copy link
Copy Markdown
Contributor Author

DimitriPapadopoulos commented Sep 27, 2021

I think codespell doesn't take punctuation into account. In any case don't bother about codespell, I can add a config file to shut it up if needed (unfortunately tox.ini won't work so it'll be setup.cfg or .codespellrc but I have requested a change in codespell-project/codespell#2086).

@DimitriPapadopoulos
Copy link
Copy Markdown
Contributor Author

The question is whether "need to delink _version.py before changing it" should be clarified for humans - not for codespell.

Comment thread NOTES Outdated
@effigies
Copy link
Copy Markdown
Contributor

Sure. Made a suggestion.

Co-authored-by: Chris Markiewicz <effigies@gmail.com>
@DimitriPapadopoulos
Copy link
Copy Markdown
Contributor Author

Perfect! Thank you.

@effigies effigies merged commit c4132a2 into python-versioneer:master Sep 27, 2021
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.

2 participants