YETUS-1202. plug-ins cannot use filefilter to build lists of files #291
Conversation
aw-was-here
force-pushed
the
fix_filters
branch
from
86aa3d4
to
1d51e07
Compare
| for i in "${HADOLINT_CHECKFILES[@]}"; do | ||
| if [[ -f "${i}" ]]; then | ||
| for i in "${CHANGED_FILES[@]}"; do | ||
| if [[ "${i}" =~ Dockerfile ]] && [[ -f "${i}" ]]; then |
There was a problem hiding this comment.
Why scope it to file names like Dockerfile? The previous values contained in HADOLINT_CHECKFILES used to be pre-filtered according to this expression?
There was a problem hiding this comment.
Yes. HADOLINT_CHECKFILES was basically everything that had Dockerfile in the name. There is a slight performance penalty not prefiltering... but only if there are no other files being excluded. As soon as exclusion is introduced, a second loop is required and that quickly makes the perf diff a negative since the exclusion list may actually be bigger than what is in the patch!
| @@ -62,7 +62,9 @@ This function call registers the `pluginname` so that test-patch knows that it e | |||
| function pluginname_filefilter | |||
| ``` | |||
|
|
|||
| defines the filefilter for the `pluginname` plug-in. | |||
| defines the filefilter for the `pluginname` plug-in. This function determines if the plug-in should | |||
There was a problem hiding this comment.
nit: capitalize "defines" at the beginning of the sentence.
| done < <("${YAMLLINT}" "${i}") | ||
| for i in "${CHANGED_FILES[@]}"; do | ||
|
|
||
| if [[ ${i} =~ \.yaml$ ]] || |
There was a problem hiding this comment.
Same question as for the Dockerfile filter previously.
|
In addition to what I wrote in the JIRA, my hunch is that longer term, individual plug-ins doing their own filename caching is likely a mistake. For example, I'm not sure what happens if a project's personality file adds files into CHANGED_FILES. There is also the general memory usage, code complexity, etc. I realize this PR doesn't really fix shellcheck and shelldocs to match the rest. But fixing those is a much bigger problem that I don't want to take on right now. |
No description provided.