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

Add forward slash to the list of characters to sanitize in sanitize_name #285

Closed
wants to merge 1 commit into from

Conversation

danielpunkass
Copy link

See Issue #32, I believe this is the correct fix for the underlying issue causing that import failure, or at least it addresses one of the possible branch naming problems that could lead to the issue.

@frej frej added the not-a-bug Fast export behaves as intended label Feb 27, 2022
@frej
Copy link
Owner

frej commented Feb 27, 2022

Please read the comment block starting the function your are modifying. It both contains an explanation for why your patch is the wrong solution and what the right solution to the problem which triggered your PR is. Besides that, / is perfectly valid in Git branch names, so there is no reason to sanitize it. Read the first item in the "Frequent Problems" section of the README for a crash course on Git branch naming.

@frej frej closed this Feb 27, 2022
@danielpunkass
Copy link
Author

Oh! I'm embarrassed to have glossed right over that comment in the sources. Thanks for explaining so patiently.

@danielpunkass danielpunkass deleted the fix-name-sanitizing branch February 27, 2022 16:00
@paulwratt
Copy link

:)

"live and learn"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not-a-bug Fast export behaves as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants