-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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 travis check for filename spaces #2821
Conversation
scripts/check_filename_spaces.sh
Outdated
! -path './Interop/Firebase Component System.md' | ||
) | ||
|
||
result=`find . -name "* *" "${options[@]}"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach will fail if there are too many files with spaces in them.
It also fails when editors create filenames with spaces, which Xcode, CMake, and CLion do regularly e.g. for temporary testing schemes. Committing this as-is would make local runs of check.sh
impossible.
It may be possible to combine this with what git considers ignored to alleviate the second concern. Let me see if I can figure it out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Using only files git knows about also enables reducing the skip list.
scripts/check_filename_spaces.sh
Outdated
! -path '*/Assets.xcassets/*' | ||
! -path '*/Target Support Files' | ||
! -path '*/Local Podspecs' | ||
! -path './Interop/Firebase Component System.md' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not rename this file rather than grandfather it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
scripts/check_filename_spaces.sh
Outdated
# Xcode-generated asset files | ||
/Assets.xcassets/ d | ||
|
||
# Grandfathered files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No files are grandfathered. Consider removing.
* Proposed improvements to space checking (#2824)
Fix #2223
Spaces in filenames cause problems with style.sh, bazel Swift rules, the command line, and potentially other scripts.
By default, ensure that new files with spaces don't get added to the repo.