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 information-type-800-60-v2r1 #764

Merged

Conversation

DimitriZhurkin
Copy link

Committer Notes

In fedramp-external-allowed-values.xml, add the information-type-800-60-v2r1 constraint.

All Submissions:

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

@DimitriZhurkin DimitriZhurkin requested a review from a team as a code owner October 8, 2024 21:03
Copy link
Contributor

@aj-stein-gsa aj-stein-gsa left a comment

Choose a reason for hiding this comment

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

Nice work. Very, very close, but I would recommend one important change.

aj-stein-gsa
aj-stein-gsa previously approved these changes Oct 9, 2024
@aj-stein-gsa
Copy link
Contributor

I am going to propose some improvements around positive and negative tests, as discussed in standup, @DimitriZhurkin, one moment.

aj-stein-gsa added a commit to aj-stein-gsa/fedramp-automation that referenced this pull request Oct 9, 2024
See the following comment for more details.

GSA#764 (comment)
aj-stein-gsa added a commit to aj-stein-gsa/fedramp-automation that referenced this pull request Oct 9, 2024
See the following comment for more details.

GSA#764 (comment)
Copy link
Contributor

@aj-stein-gsa aj-stein-gsa left a comment

Choose a reason for hiding this comment

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

Let's expand some of the tests and discuss changes in my recommendations in the PR (I cannot use suggestions for files out of scope for the PR).

DimitriZhurkin#4

I DMed in the team chat to further discuss the implications here. I want more robust tests based on other bugs I have seen reported and want to make sure we fully test the mechanics in positive and negative tests if possible.

@aj-stein-gsa aj-stein-gsa self-requested a review October 9, 2024 18:01
@DimitriZhurkin
Copy link
Author

I tried your suggestions, and ssp-all-VALID.xml was failing. I removed the changes from it.

I'll commit the changes to ssp-information-type-id-INVALID.xml.

@aj-stein-gsa
Copy link
Contributor

I tried your suggestions, and ssp-all-VALID.xml was failing. I removed the changes from it.

It took me some time to figure out why but I read through the output and I understand now. I will complete my review on this request.

@aj-stein-gsa aj-stein-gsa merged commit 9d50039 into GSA:develop Oct 10, 2024
5 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.

2 participants