-
Notifications
You must be signed in to change notification settings - Fork 219
Sort the results of a filesearch #1214
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
Conversation
|
Refer to this link for build results (access rights to CI server needed): |
|
@boegel Jenkins says: |
|
retest this please |
easybuild/tools/filetools.py
Outdated
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.
maybe better use hits = sorted(...), wherever hits is defined?
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? Does that make any difference?
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.
it ensures that hits is always sorted, may be used deeper down or higher up (maybe not now, but later, who knows)
also, using .sort() means side-effects (hits is modified in-place, no explicit reassignment to hits), which makes it less easier to understand the code (it may be more efficient, but that'll be in the noise here, most likely)
|
@boegel fixed |
easybuild/tools/filetools.py
Outdated
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.
isn't hits defined higher up? do the sorted thing there, outside of this if hits block
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? it only makes sense to sort if there is something to sort?
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.
again, hits may be used again deeper down or higher up, who knows
sorted([]) is pretty cheap anyway ;)
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
going in, sorry for being so picky @wpoely86 ;) |
Sort the results of a filesearch
Seems more logical to me and it shouldn't break anything...