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.

Sign up for GitHub

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

Jump to bottom

Remove @ in temporary directories #2060

Merged
merged 2 commits into from
Nov 28, 2015
Merged

Remove @ in temporary directories #2060

merged 2 commits into from
Nov 28, 2015

Conversation

Utsav2
Copy link
Contributor

@Utsav2 Utsav2 commented Nov 27, 2015

Should fix #1650

@sheerun
Copy link
Contributor

sheerun commented Nov 27, 2015

Can you write a test to confirm it?

@sheerun
Copy link
Contributor

sheerun commented Nov 27, 2015

Maybe it would be better to mp5 the _name?

@Utsav2
Copy link
Contributor Author

Utsav2 commented Nov 27, 2015

  1. What do you mean by mp5? Are you saying that I should run the replace() only on _name instead of the entire path?

  2. I've written a test to check that the '@' character is not preserved in directory name. What else should the test check?

@sheerun
Copy link
Contributor

sheerun commented Nov 27, 2015

  1. Use md5 hash of this._name instead of this._name itself
  2. It should be ok

@Utsav2
Replace temp directory name with md5 hash
852a586 
Fixes issues with characters like @ interfering with directory name
lookups
@Utsav2
Copy link
Contributor Author

Utsav2 commented Nov 27, 2015

I've made the change, let me know if I should squash both the commits together

@sheerun
Copy link
Contributor

sheerun commented Nov 28, 2015

Looks good. Thank you! :)

sheerun added a commit that referenced this pull request Nov 28, 2015
@sheerun
Merge pull request #2060 from Utsav2/remove-at
107ad1d 
Remove @ in temporary directories
@sheerun sheerun merged commit 107ad1d into bower:master Nov 28, 2015
@Utsav2 Utsav2 deleted the remove-at branch December 3, 2015 03:40
@Utsav2 Utsav2 mentioned this pull request Dec 16, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

When using svn with bower and temp dir has @ the repo is not fetched correctly
3 participants
@Utsav2 @sheerun @faceleg