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

Correct digest lengths #2068

Conversation

david-waltermire
Copy link
Contributor

@david-waltermire david-waltermire commented Nov 11, 2024

Committer Notes

Based on some testing, the required digest lengths should be twice their current size to be correct. This PR adjusts the values to be the correct size.

Resolves #2053

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.

@david-waltermire david-waltermire requested a review from a team as a code owner November 11, 2024 02:49
@aj-stein-gsa
Copy link
Contributor

@david-waltermire, per our desire to coordinate a patch release with NIST, can we please target this PR on main, not develop? I will ping you in an upcoming discussion topic. Thanks!

@wendellpiez
Copy link
Contributor

That would be fine with me (main not develop) although I just stated the opposite (over on #2069) - shows what I know.

As for the merits of the PR, 👍 to backward-compatible improvements and corrections - if the constraint is to be included at all it should be correct. (Removing it into a different layer of constraint checking is also a good solution in my view.)

@david-waltermire
Copy link
Contributor Author

According to the contributing guidelines it looks like I should target release-1.1 for a patch release. I am going to rebase and change the PR to point there.

@david-waltermire david-waltermire force-pushed the feature/issue2053-correct-digest-lengths branch from 5298fce to 4019d57 Compare November 13, 2024 23:41
@david-waltermire david-waltermire changed the base branch from develop to release-1.1 November 13, 2024 23:41
@iMichaela
Copy link
Contributor

According to the contributing guidelines it looks like I should target release-1.1 for a patch release. I am going to rebase and change the PR to point there.

This is correct and everything we merged already in develop was staged for the v1.1.3 patch release, hence the request to submit the PRs towards develop branch, so we do not need to rebase develop to release 1.1.3. There are no new features or models added, but dependency updates are also a needed. There is one SAP constraints bug #2059 that must be also fixed with the v1.1.3. Once we decide the direction to go for #2059, will implement it, asap, test it and release 1.1.3.

@david-waltermire
Copy link
Contributor Author

According to the contributing guidelines it looks like I should target release-1.1 for a patch release. I am going to rebase and change the PR to point there.

This is correct and everything we merged already in develop was staged for the v1.1.3 patch release, hence the request to submit the PRs towards develop branch, so we do not need to rebase develop to release 1.1.3. There are no new features or models added, but dependency updates are also a needed. There is one SAP constraints bug #2059 that must be also fixed with the v1.1.3. Once we decide the direction to go for #2059, will implement it, asap, test it and release 1.1.3.

I am happy to rebase to develop if that is what you want me to do.

I do want to point out that what you are suggesting is not consistent with the practices for a patch release identified in the contributing guidelines for the repository which states:

Contributions can be made to the following branches in this repository:

  • release-*: The release branches are used to provide patches to a major or minor version of OSCAL. The branches are named release-major.minor. You should provide changes only to the highest numbered minor release for a given major release. Patch releases are made more frequently than major or minor releases.
  • develop: This branch is used to queue changes for the next major/minor release of OSCAL. A major/minor release will result in the creation of a new release branch, once the development has been completed and the update is to be staged for release.

Inconsistent application of these guidelines makes the process confusing, difficult to predict, and more labor intensive for contributors. It would be helpful to have a more consistent approach.

@david-waltermire david-waltermire force-pushed the feature/issue2053-correct-digest-lengths branch from 4019d57 to 728af69 Compare November 14, 2024 18:11
@david-waltermire david-waltermire changed the base branch from release-1.1 to develop November 14, 2024 18:12
Copy link
Contributor

@iMichaela iMichaela left a comment

Choose a reason for hiding this comment

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

@iMichaela iMichaela merged commit 179a69b into usnistgov:develop Nov 14, 2024
1 check passed
@david-waltermire david-waltermire deleted the feature/issue2053-correct-digest-lengths branch November 14, 2024 22:04
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.

4 participants