Chore: small opt to improve readability #10241
Conversation
|
I see configs are being set in reverse order (from their current setting), but I don't see a corresponding change in whichever module is reading from |
|
not sure why reverse order -- the change was just as-is. |
| @@ -209,7 +209,7 @@ class Config { | |||
| const localConfigHierarchyCache = this.configCache.getHierarchyLocalConfigs(localConfigDirectory); | |||
|
|
|||
| if (localConfigHierarchyCache) { | |||
| const localConfigHierarchy = localConfigHierarchyCache.concat(configs.reverse()); | |||
| const localConfigHierarchy = localConfigHierarchyCache.concat(configs); | |||
|
|
|||
| this.configCache.setHierarchyLocalConfigs(searched, localConfigHierarchy); | |||
| return localConfigHierarchy; | |||
There was a problem hiding this comment.
I'm confused about this part of the change. Since localConfigHierarchy is returned here, it seems like this change will affect the behavior of the function, because localConfigHierarchy will have some configs in the wrong order. Is this intentional?
There was a problem hiding this comment.
I'm not sure it's wrong order -- I replaced all configs.push(localConfig); => configs.unshift(localConfig); , so configs could always be reverse (same ) order as before. am I missing something?
There was a problem hiding this comment.
Since configs is used on line 212 instead of configs.reverse(), the configs will be stored in the cache in the opposite order from before.
This isn't necessarily a problem if this function is the only function that reads from that cache, but it could cause a problem if there are other parts of the code that read from the cache and expect the reverse order.
There was a problem hiding this comment.
oh, sorry for misleading. Actually configs would be always reverse order, so the line 212 is the same as before configs.reverse().
|
@not-an-aardvark Are your concerns addressed here? |
|
Thanks for contributing! |
eslint-deprecated
bot
added
the
archived due to age
What is the purpose of this pull request? (put an "X" next to item)
[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[x] Other, please explain:
What changes did you make? (Give an overview)
use
config.unshift()to avoid callingconfig.reverse()-- more clear, and a little perf win.Is there anything you'd like reviewers to focus on?
no