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

Refactoring the expanded method in the paths module. #10018

Closed

Conversation

wangjohn
Copy link
Contributor

I'm changing the expanded method to remove an unnecessary sort. This sort was called on files from directories in the expanded file path. The sort shouldn't matter because the method simply returns all unique values of the expanded file path.

I've also refactored the rest of the code a little bit. In particular, I am using map instead of concatenating to an array.

@josevalim
Copy link
Contributor

The sort comes from this commit: 5c8be9e

Unfortunately, I have no idea why it is required and the commit does not explain so.

@wangjohn
Copy link
Contributor Author

@josevalim I believe I've tracked the sort down to this issue: #2569, although I'm not sure if the sort is directly relevant because the fix was made in the hike gem.

The problem was that md5 fingerprints were different in development vs production because the dir file globs weren't sorted in the hike gem's entries method.

This makes me wonder if the sort in the expanded method actually is required since it seems like the fix was only required in the hike gem. @tenderlove do you remember if the sort in the expanded method was required to fix #2569? If so, I'll add some documentation as to why sort is called, otherwise I'll get rid of it.

@wangjohn wangjohn closed this Jun 3, 2013
@wangjohn wangjohn deleted the refactoring_expanded_for_paths branch June 3, 2013 00:07
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.

None yet

2 participants