Conversation
markchalloner
left a comment
markchalloner
left a comment
There was a problem hiding this comment.
Thanks, it's looking good just a couple of minor suggestions.
Can you also update the CHANGELOG file with the changes from this PR and your previous one?
| if [ -d "${plugin_dir}" ] | ||
| then | ||
| find "${plugin_dir}" -maxdepth 1 -type f -exec echo "${plugin_type},{}" \; | ||
| plugins=() |
There was a problem hiding this comment.
Is this variable needed? I can't see it being referenced in the following code.
There was a problem hiding this comment.
Actually I realise this may have been to cache the result of the find. That would save a command.
There was a problem hiding this comment.
I think that might actually be a mistake. I don't see it in the master branch of this repo, and I added it on the Fixed MacOS compatibility commit for no apparent reason. And, if it actually does any caching, I can't see how, but I'm happy to leave it there if it's useful.
| then | ||
| find "${plugin_dir}" -maxdepth 1 -type f -exec echo "${plugin_type},{}" \; | ||
| plugins=() | ||
| for plugin in $(find "${plugin_dir}" -maxdepth 1 -type f -exec basename {} \; | grep "^[0-9]" | LC_ALL="C" sort -g) |
There was a problem hiding this comment.
Probably worth a quick comment here as to why the loop is run twice.
There was a problem hiding this comment.
That loop is ran twice to first echo the numeric prefixed plugins (note that the output is piped to a grep) and then echo the not numeric ones. This is basically what you suggested doing in this comment.
As you also said, it's hacky, but I really couldn't come with a better solution :/
2 participants
As discussed in #36, this code sorts plugins by number before running them.
Please let me know of any improvements you see on this code, and thanks in advance for reviewing.