Skip to content
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

Add imported-from link rel to relevant SSP assemblies #1403

Conversation

aj-stein-nist
Copy link
Contributor

@aj-stein-nist aj-stein-nist commented Aug 4, 2022

Committer Notes

{Please provide a brief description of what this PR accomplishes. Be sure to reference any issues addressed. If the PR is a work-in-progress submitted for early review, please include [WIP] at the beginning of the title or mark the PR as DRAFT.}

All Submissions:

By submitting a pull request, you are agreeing to provide this contribution under the CC0 1.0 Universal public domain dedication.

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you updated all OSCAL website and readme documentation affected by the changes you made? Changes to the OSCAL website can be made in the docs/content directory of your branch.
  • Have you written new tests for your core changes, as applicable?
  • Have you included examples of how to use your new feature(s)?

@aj-stein-nist aj-stein-nist changed the base branch from main to develop August 4, 2022 17:38
@aj-stein-nist aj-stein-nist self-assigned this Aug 4, 2022
@aj-stein-nist aj-stein-nist force-pushed the 1023-add-imported-from-relation-to-links-in-ssp branch from 71f4454 to 3f8e1c5 Compare August 4, 2022 18:26
@aj-stein-nist aj-stein-nist linked an issue Aug 4, 2022 that may be closed by this pull request
5 tasks
@aj-stein-nist aj-stein-nist added the Scope: Modeling Issues targeted at development of OSCAL formats label Aug 4, 2022
@aj-stein-nist
Copy link
Contributor Author

OK per the updated dependencies listing, I am gonna park this until #756 is implemented and circle back to this. Moving the linked issue in the TODO column.

Copy link
Contributor

@david-waltermire david-waltermire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please relocate the constraint for component/link/@rel to the system-component definition.

@aj-stein-nist
Copy link
Contributor Author

Please relocate the constraint for component/link/@rel to the system-component definition.

So I pushed up a separate commit 497be0d and didn't squash it yet. I understand the request, but I am not sure how this will not lead to undesirable behavior. Can you explain to me when you have a chance?

If we move it here, does this not in fact allow this prop on any any component, not only a component instance in a SSP? Do we want abstracted component-definition(s) component(s) able to have this prop and potentially lead to confusion? Let me know what I am missing.

Copy link
Contributor

@david-waltermire david-waltermire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@aj-stein-nist aj-stein-nist changed the title [WIP] Add imported-from link rel to relevant SSP assemblies Add imported-from link rel to relevant SSP assemblies Aug 11, 2022
@aj-stein-nist aj-stein-nist marked this pull request as ready for review August 11, 2022 18:24
@aj-stein-nist
Copy link
Contributor Author

The failed CI job was the link checker from cache errors with the W3C website docs, it worked after re-run. @david-waltermire-nist, per discussion during the weekly status meeting, this is ready for review.

@aj-stein-nist
Copy link
Contributor Author

Please relocate the constraint for component/link/@rel to the system-component definition.

So I pushed up a separate commit 497be0d and didn't squash it yet. I understand the request, but I am not sure how this will not lead to undesirable behavior. Can you explain to me when you have a chance?

If we move it here, does this not in fact allow this prop on any any component, not only a component instance in a SSP? Do we want abstracted component-definition(s) component(s) able to have this prop and potentially lead to confusion? Let me know what I am missing.

Also just rebased down to one commit, as the feedback concern in #1403 (review) has been explained and addressed by Dave.

@david-waltermire
Copy link
Contributor

@aj-stein-nist This PR needs to be rebased on the latest develop.

@aj-stein-nist aj-stein-nist force-pushed the 1023-add-imported-from-relation-to-links-in-ssp branch from 37a6830 to 923fa0a Compare August 25, 2022 12:28
@aj-stein-nist
Copy link
Contributor Author

@aj-stein-nist This PR needs to be rebased on the latest develop.

Rebased.

@aj-stein-nist aj-stein-nist force-pushed the 1023-add-imported-from-relation-to-links-in-ssp branch 4 times, most recently from 36ab333 to fcdfcdd Compare August 25, 2022 14:33
@aj-stein-nist aj-stein-nist force-pushed the 1023-add-imported-from-relation-to-links-in-ssp branch from fcdfcdd to 3af9f8c Compare August 25, 2022 14:39
README_validations.md Outdated Show resolved Hide resolved
@aj-stein-nist aj-stein-nist marked this pull request as ready for review August 25, 2022 14:53
@aj-stein-nist aj-stein-nist force-pushed the 1023-add-imported-from-relation-to-links-in-ssp branch 2 times, most recently from 1eb2bed to c4377f0 Compare August 25, 2022 15:11
@aj-stein-nist
Copy link
Contributor Author

Also TIL git commit --amend --signoff is needed to sign-off commits that are co-authored by team members. Now the PR commits are not so unverified.

@aj-stein-nist aj-stein-nist changed the base branch from develop to feature-metadata-actions-assembly August 25, 2022 18:42
@aj-stein-nist
Copy link
Contributor Author

aj-stein-nist commented Aug 25, 2022

@david-waltermire-nist, I pointed this PR to a feature branch as instructed during the weekly sync for when we are ready to merge. Woops, this was meant for a totally different PR, disregard! 😆

@aj-stein-nist aj-stein-nist changed the base branch from feature-metadata-actions-assembly to develop August 25, 2022 18:48
@aj-stein-nist aj-stein-nist force-pushed the 1023-add-imported-from-relation-to-links-in-ssp branch 3 times, most recently from 02ae622 to eeb54c1 Compare August 25, 2022 20:52
Co-authored-by: David Waltermire <[email protected]>
Signed-off-by: Alexander Stein <[email protected]>
@aj-stein-nist aj-stein-nist force-pushed the 1023-add-imported-from-relation-to-links-in-ssp branch from 60496ae to c3a8a9c Compare August 29, 2022 21:32
@aj-stein-nist
Copy link
Contributor Author

Rebased and waiting for pending feedback, but minor recommendation in #1403 (comment) was added before rebase. Should be good to go.

Copy link
Contributor

@david-waltermire david-waltermire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@david-waltermire david-waltermire merged commit 054a7b9 into usnistgov:develop Aug 31, 2022
aj-stein-nist added a commit to aj-stein-nist/OSCAL-forked that referenced this pull request Oct 6, 2022
Co-authored-by: David Waltermire <[email protected]>
Signed-off-by: Alexander Stein <[email protected]>
aj-stein-nist added a commit that referenced this pull request Oct 18, 2022
Co-authored-by: David Waltermire <[email protected]>
Signed-off-by: Alexander Stein <[email protected]>
david-waltermire added a commit that referenced this pull request Oct 31, 2022
Co-authored-by: David Waltermire <[email protected]>
Signed-off-by: Alexander Stein <[email protected]>
aj-stein-nist added a commit to aj-stein-nist/OSCAL-forked that referenced this pull request Jan 10, 2023
Co-authored-by: David Waltermire <[email protected]>
Signed-off-by: Alexander Stein <[email protected]>
aj-stein-nist added a commit to aj-stein-nist/OSCAL-forked that referenced this pull request Feb 6, 2023
Co-authored-by: David Waltermire <[email protected]>
Signed-off-by: Alexander Stein <[email protected]>
aj-stein-nist added a commit to aj-stein-nist/OSCAL-forked that referenced this pull request Jun 29, 2023
Co-authored-by: David Waltermire <[email protected]>
Signed-off-by: Alexander Stein <[email protected]>
aj-stein-nist added a commit to aj-stein-nist/OSCAL-forked that referenced this pull request Jul 10, 2023
Co-authored-by: David Waltermire <[email protected]>
Signed-off-by: Alexander Stein <[email protected]>
aj-stein-nist added a commit to galtm/OSCAL that referenced this pull request Sep 28, 2023
Co-authored-by: David Waltermire <[email protected]>
Signed-off-by: Alexander Stein <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Modeling Issues targeted at development of OSCAL formats
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add imported-from relation to links in SSP
3 participants