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

Normalize "SBOM" to "Sbom" through most of the code base #950

Merged
merged 207 commits into from
Mar 1, 2025

Conversation

DaveTryon
Copy link
Contributor

@DaveTryon DaveTryon commented Feb 27, 2025

(Note: This replaces #947, which got into a state where the base was missing many of the changes from #924)

As called out in #685, our code is very inconsistent in its usage of "SBOM" and "Sbom". Older files seemed to start with "SBOM", then migrated somewhat toward "Sbom", but we have a bit of a mix. We punted on #685 because of compatibility concerns.

As we've discussed separately, SPDX 3.0 is a breaking change, so we have a window of opportunity to standardize our API. This PR is an attempt to name that happen, following the following rules:

  1. The new standard is to follow PascalCasing
  2. File names and class names are case-consistent
  3. Let Visual Studio automate as many of the changes as possible
  4. Make the commits as tightly scoped as possible
  5. Try to keep the docs in sync with the code changes
  6. To keep things clean, use "Sbom" in all code constructs, with 2 exceptions to come later:
  7. Files that include "SPDX" in the name (minimize merge conflicts with API changes to support SPDX 3.0 #924
  8. Leave the test files alone where possible. I think I renamed 1 test file by mistake, but I can back that out if we want
  9. Since I'm working on Windows, which ignores case differences, the file renames were a 2-step process:
  10. Rename SBOMFoo to SBOMFoo_, allowing Visual Studio to update all usages
  11. Rename SBOMFoo_ to SbomFoo, allowing Visual Studio to update all usages
  12. Build and run tests frequently along the way to make sure that we don't perpetuate breakage
  13. Search for SBOM\w\w using regular expressions as a way to find cases that may have been missed

The PR itself is broad (93 files), but most of the review should be trivial.

Places where a case-sensitive grep for SBOM\w\w finds hits:

  • Our ProductName is still "MIcrosoft.SBOMTool" - we'll probably need to keep this for backward compatibility with tools that consume our SBOM files
  • Our docs use a namespace of SBOMApiExample - trivial change that won't impact the API
  • Many tests still use SBOM in class and file names - will fix later as they don't impact the API

Proposed roadmap:

  1. Merge API changes to support SPDX 3.0 #924
  2. Update this PR to reflect changes from API changes to support SPDX 3.0 #924
  3. Merge this PR
  4. Decide if we want to normalize "SPDX" to "Spdx". If so, create a similar PR
  5. Create a migration document from API 3.x to 4.0 (we need do this even if we choose not to normalize the casing)
  6. Normalize casing in test names (doesn't impact the API, could happen after we release the 4.0 API)

@ZhengHong-Tan
Copy link

Just curious, how do you test that these refactors don't cause any regression issues? or more generally, how would we test sbom-tool? thanks!

@microsoft microsoft deleted a comment from github-actions bot Feb 27, 2025
@DaveTryon
Copy link
Contributor Author

Just curious, how do you test that these refactors don't cause any regression issues? or more generally, how would we test sbom-tool? thanks!

Class functionality is pinned by the unit tests. CLI functionality is pinned by the end-to-end tests. The API will break, but we already had to break it to add the SPDX 3.0 support. The new bits will be released with a major version bump. We're taking advantage of that opportunity to normalize the interface casing. We'll write a migration document to help guide API consumers through the changes that they will need to implement when moving to the new version. Our internal repo will also need to be updated when we pick up this version, so we'll have something of a validation for the migration guide (which can be updated as needed, without needing to change the API again)

@DaveTryon
Copy link
Contributor Author

/azp run

Copy link

This PR changes files in the API project. Does it change any of the API interfaces in any way? Please note that this includes the following types of changes:

  • Changing the signature of an existing interface method
  • Adding a new method to an existing interface
  • Adding a required data member to a class that an existing interface method consumes

Because any of these changes can potentially break a downstream consumer with customized interface implementations, these changes need to be treated as breaking changes. Please do one of the following:

Option 1 - Publish this as a breaking change

  1. Update the documentation to show the new functionality
  2. Bump the major version in the next release
  3. Be sure to highlight the breaking changes in the release notes

Option 2 - Refactor the changes to be non-breaking

  1. Review this commit, which adds a new interface in a backward-compatible way
  2. Refactor the change to follow this pattern so that existing interfaces are left completely intact
  3. Bump the minor version in the next release

@microsoft microsoft deleted a comment from github-actions bot Feb 27, 2025
@microsoft microsoft deleted a comment from github-actions bot Feb 27, 2025
@DaveTryon
Copy link
Contributor Author

/azp run

@DaveTryon DaveTryon merged commit d867abc into main Mar 1, 2025
5 checks passed
@DaveTryon DaveTryon deleted the DaveTryon/pascal-casing-for-sbom branch March 1, 2025 05:10
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.

3 participants