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

feat: deprecate axillary public packages in favor of private versions #1309

Merged
merged 5 commits into from
Oct 18, 2024

Conversation

G-Rath
Copy link
Collaborator

@G-Rath G-Rath commented Oct 8, 2024

As part of the upcoming v2 release we're planning on making as many of our currently-public packages that we can private, in order to make it easier for us to maintain them since most are "axillary" packages that only expect to be used by the scanner, and that it's not a breaking change to make particular packages public again in future if that does prove useful.

While we cannot remove the existing public packages until v2 is released, we can still create the private versions and switch over to using them - this means that we can start making what'd be breaking changes to their interfaces, and also gives people a chance to report their usage of a particular package due to them now being marked as deprecated.

Of our currently public packages, these are being made private and marked as deprecated:

  • config
  • depsdev
  • grouper
  • spdx

Of the others:

  • lockfile will be replaced with https://github.com/google/osv-scalibr, but we won't be marking it as deprecated until @another-rex has finished migrating the implementations here over to that library
  • models is going to be broken up, with at least the constants being moved to a dedicated package in https://github.com/ossf/osv-schema and at least some of the other content moved "elsewhere" (though the exact fate of everything is yet to be determined)
  • osvscanner is purposely public and will remain that way, which also necessitates reporter being public since it's a parameter to the API functions so it'd be burdensome for consumers to have it private
  • osv is used by osvscanner, but technically might not need to public; but that is still I think something we're interested in keeping public at some point - I believe there are talks of oneday maybe housing it in https://github.com/google/osv.dev, but for now it can remain public too

Note this is focused on just "moving" the existing packages - there are already a few functions in e.g. config that are marked as deprecated, which I'll remove in a follow-up pull request to make it easier to review.

@codecov-commenter
Copy link

codecov-commenter commented Oct 8, 2024

Codecov Report

Attention: Patch coverage is 90.87452% with 24 lines in your changes missing coverage. Please review.

Project coverage is 68.07%. Comparing base (9cb6791) to head (1c7fa1f).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
internal/depsdev/license.go 61.29% 18 Missing and 6 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1309      +/-   ##
==========================================
- Coverage   68.24%   68.07%   -0.18%     
==========================================
  Files         178      183       +5     
  Lines       17234    17498     +264     
==========================================
+ Hits        11761    11911     +150     
- Misses       4833     4947     +114     
  Partials      640      640              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@G-Rath G-Rath force-pushed the move-packages branch 2 times, most recently from 253cc91 to 73b9fcb Compare October 8, 2024 20:34
@G-Rath
Copy link
Collaborator Author

G-Rath commented Oct 8, 2024

I've confirmed locally that we've no longer got any unexpected dependencies on the deprecated packages by deleting the public versions of the packages being made private, and everything still builds and passes etc

@G-Rath G-Rath marked this pull request as ready for review October 8, 2024 20:41
Copy link
Contributor

@cuixq cuixq left a comment

Choose a reason for hiding this comment

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

LGTM, just one question - we have a v2 branch which I think is for v2 stuff, especially for this sort of breaking change. @another-rex wdyt?

@G-Rath
Copy link
Collaborator Author

G-Rath commented Oct 8, 2024

This isn't a breaking change, which is the point - it's breaking to remove the packages, but not for us to stop using them; once this has been landed, then you can rebase osv-scanner-v2 onto main and then actually delete the public packages

cuixq pushed a commit that referenced this pull request Oct 9, 2024
While working on #1309 I learned that this list is out of date, so this
regenerates it - I don't think it's a big deal, but I've also made a
local note to look into potentially automating this more (it probably
ideally should happen before every release)
internal/grouper/grouper.go Outdated Show resolved Hide resolved
internal/grouper/grouper.go Outdated Show resolved Hide resolved
@another-rex another-rex enabled auto-merge (squash) October 18, 2024 01:41
@another-rex another-rex merged commit a7d6524 into google:main Oct 18, 2024
13 checks passed
@another-rex another-rex deleted the move-packages branch October 18, 2024 01:44
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