GH2524: Added support for XSL arguments in XmlTransform #2589
Conversation
|
Just curious, did I something wrong with this pull request? Please tell me... |
|
@deqenq sorry, you've done nothing wrong, current status it's on the Cake team to fully review this, which we've unfortunately not gotten around to yet. |
augustoproiete
changed the title
|
Should I try to resuscitate this pull request or not? That's the question. |
augustoproiete
force-pushed
the
gh-2524
branch
2 times, most recently
from
6461ac9
to
a307768
Compare
gep13
changed the title
There was a problem hiding this comment.
@deqenq sorry for this late feedback. The changes look good.
However, there are - in addition to the static XmlTransformation.Transform() methods - also corresponding overloads in XmlTransformationAlias to be used as extensions to the ICakeContext.
Are you in a position to add those, as well?
Also, I would much rather prefer to have the XsltArgumentList as a property to the XmlTransformationSettings instead of having another set of overloads.
nils-a
added
Waiting for issuer feedback
and removed
Response from Cake team required
labels
|
@nils-a Thank you for your suggestions, I will try to fix it. |
- XsltArgumentList can be null according to the documentation - fixed tests - fixed exception messages to be aligned to documentation - no need to overloaded aliases
- XsltArgumentList can be null according to the documentation - fixed tests - fixed exception messages to be aligned to documentation - no need to overloaded aliases
|
@deqenq your changes have been merged, thanks for your contribution 👍 |
fixes #2524
I added two overloaded XmlTransform methods that accept XsltArgumentList as arguments for XSL tranformation. Also new unit tests were added - I tried to use existing input/output sample data as much as possible so there are only two new XSL definitions (with arguments) needed.