Enhance CompilationDir to filter out cases outlined in #187 & #188#190
Merged
kpaulisse merged 2 commits intogithub:masterfrom Dec 9, 2018
Merged
Conversation
Contributor
Author
|
Oops, lot of red. Looks like I missed something :) |
added 2 commits
June 6, 2018 21:04
…ltiple values There were two cases where CompilationDir wasn't filtering out changes: * if the parameter value is an arbitrary data structure (ie hash, array or mix of both) * if the parameter value is a string containing more than one occurence of the compilation dir It turns out that both can be fixed by just replacing the compilation dirs in both the new and old values with empty strings and comparing what's left. It's probably much slower than the original string-only implementation but covers much more cases as demonstrated by github#187 and github#188.
I noticed a few mistakes while reading the API documentation, and I couldn't resist to fix them.
masterzen
force-pushed
the
bugfix/fix-compilatior-dir-arrays
branch
from
4aa1a8f to
f1039dc
Compare
Merged
kpaulisse
reviewed
Dec 9, 2018
Contributor
kpaulisse
left a comment
kpaulisse
left a comment
There was a problem hiding this comment.
Thank you for submitting this PR, and I apologize for the delay in my response. This will be included in octocatalog-diff release 1.5.4 later this week. I sincerely appreciate your contribution!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We were having some compilation dir differences in our catalog diff because of several use cases that weren't covered by the current
CompilationDir.This change implements a different way of checking for the compilation dir so that
CompilationDirsupports complex parameter values (ie nested hashes/arrays) and/or strings containing the compilation dir more than once.To be noted that the chosen implementation might be much slower than the original algorithm because it defensively dups the parameter values. This might be problematic, and I'm open to suggestion on how to better implement this feature.
As a bonus I'm including a small minor fix of the API documentation of the diff object :)