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

1797 allow local definition of asset type prop values #1821

Merged

Conversation

Arminta-Jenkins-NIST
Copy link
Contributor

@Arminta-Jenkins-NIST Arminta-Jenkins-NIST commented Jun 27, 2023

Committer Notes

asset-type will now allow-other values making the property less strictive

{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.

(For reviewers: The wiki has guidance on code review and overall issue review for completeness.)

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 written new tests for your core changes, as applicable?
  • Have you included examples of how to use your new feature(s)?
  • 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.

Copy link
Contributor

@wendellpiez wendellpiez left a comment

Choose a reason for hiding this comment

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

It looks like a bunch of other changes are committed to this PR - it should be probably be targeting develop or another branch, not main? (@aj-stein-nist can advise)

@aj-stein-nist
Copy link
Contributor

@Arminta-Jenkins-NIST did you push up the commit after editing the branch? I can't rebase to develop because this branch is the same as develop, perhaps you're missing the commit where you edited the requisite files?

@aj-stein-nist aj-stein-nist changed the base branch from main to develop June 28, 2023 17:30
Copy link
Contributor

@aj-stein-nist aj-stein-nist left a comment

Choose a reason for hiding this comment

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

Thanks for pushing this up and doing a quick synchronous review. I provided some changes I would recommend.

src/metaschema/oscal_component_metaschema.xml Outdated Show resolved Hide resolved
src/metaschema/oscal_implementation-common_metaschema.xml Outdated Show resolved Hide resolved
src/metaschema/oscal_implementation-common_metaschema.xml Outdated Show resolved Hide resolved
@aj-stein-nist
Copy link
Contributor

@Arminta-Jenkins-NIST tomorrow I am going to have to help you reconfigure this branch on your computer. I will rebase it for you now.

@aj-stein-nist aj-stein-nist force-pushed the 1797-allow-local-definition-of-asset-type-prop-values branch from e15d513 to b057dae Compare June 29, 2023 21:30
@aj-stein-nist
Copy link
Contributor

Hi @Arminta-Jenkins-NIST, can you confirm if you made changes locally but forgot to push them up? I see you 👍ed some comments and resolved them, but changes are not in the PR, so it is possible you did not do git push origin HEAD or similar after pushing the changes locally. Let me know today if you need help.

@Arminta-Jenkins-NIST
Copy link
Contributor Author

@aj-stein-nist I thought I did everything properly. However, yesterday's peering session with @iMichaela on issue #1798 showed there are issues with my local setup. It is still unclear to me what went wrong.

@aj-stein-nist aj-stein-nist self-requested a review July 7, 2023 18:20
@aj-stein-nist aj-stein-nist force-pushed the 1797-allow-local-definition-of-asset-type-prop-values branch from 021d94d to 3dca6c0 Compare July 7, 2023 18:27
@aj-stein-nist aj-stein-nist force-pushed the 1797-allow-local-definition-of-asset-type-prop-values branch from 3dca6c0 to 6cdbf59 Compare July 11, 2023 12:44
@aj-stein-nist aj-stein-nist merged commit bc7efd3 into develop Jul 11, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow local definition of "asset-type" prop values
3 participants