-
Notifications
You must be signed in to change notification settings - Fork 183
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support additional control-origination props #784 #1460
Support additional control-origination props #784 #1460
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like #1456, I'm A.J. and I approve this message. :-)
What is the value of a prop with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to think more about the value side of this new property.
@david-waltermire-nist Are we wanting to review this list?
|
Co-authored-by: David Waltermire <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It appears I missed this bit, but Dave pointed it out and you merged it in, so now I can approve again with more confidence we met reqs. :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I am going to be that guy and retract my approval so we can discuss something? Let me know when you have time to discuss, Chris (and/or others maybe).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Back to my previous state. Will approve this PR and table further conversation of follow-on work until #1502.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks!
* Support additional control-origination props #784 * Update recommended path to target additional nodes. Co-authored-by: David Waltermire <[email protected]>
* Support additional control-origination props #784 * Update recommended path to target additional nodes. Co-authored-by: David Waltermire <[email protected]>
…#1460) * Support additional control-origination props usnistgov#784 * Update recommended path to target additional nodes. Co-authored-by: David Waltermire <[email protected]>
…#1460) * Support additional control-origination props usnistgov#784 * Update recommended path to target additional nodes. Co-authored-by: David Waltermire <[email protected]>
…#1460) * Support additional control-origination props usnistgov#784 * Update recommended path to target additional nodes. Co-authored-by: David Waltermire <[email protected]>
…#1460) * Support additional control-origination props usnistgov#784 * Update recommended path to target additional nodes. Co-authored-by: David Waltermire <[email protected]>
…#1460) * Support additional control-origination props usnistgov#784 * Update recommended path to target additional nodes. Co-authored-by: David Waltermire <[email protected]>
Committer Notes
Based on the request in #784, extended the xpath to include control-origination props for:
The updated xpath selected the following paths from my test case:
A note was added that the child context will override the parent
control-origination
.