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.

Sign up for GitHub

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

Jump to bottom

Unify Ecosystem type and constants #442

Open
another-rex opened this issue Jul 13, 2023 · 1 comment
Open

Unify Ecosystem type and constants #442

another-rex opened this issue Jul 13, 2023 · 1 comment
Labels
V2 Wishlist Enhancements that require a breaking change

Comments

@another-rex
Copy link
Collaborator

The current Ecosystem type and constants are duplicated in multiple packages, and in multiple places ecosystem also only has a type of string. For V2 of the API we should combine these to the same ecosystem type.
@another-rex another-rex added the V2 Wishlist Enhancements that require a breaking change label Jul 13, 2023
@another-rex another-rex changed the title Unify Ecosystem Constants Unify Ecosystem type and constants Jul 13, 2023
@another-rex another-rex mentioned this issue Aug 28, 2023
Merged
another-rex added a commit that referenced this issue Aug 28, 2023
@another-rex
Refactor models (#510)
5a316f5 
Pulled refactor out to a separate PR as suggested here: #505 

In this PR:
- Refactored models package to no longer depend on lockfile
- Moved functions that required lockfile types into separate internal
package under `utility/vulns`
- Copied over OSV url from `osv` package to avoid dependency on `osv`
from `output`/`reporter`

I went through and graphed out the dependencies, 

- `models` no longer depends on any internal dependencies
- `lockfile` doesn't depend on any internal dependencies apart from
`cacheexp`
- `osv` brings in both `lockfiles` and `models`. This is probably the
most problematic one. Ideally `osv` should only be referencing `models`,
but it'll be a breaking change to change this API. This is one of the
main things we should change for V2 (related to #442) (TODO: Make
separate issue for this
- `reporter` depends on the internal `output`, which depends on `osv`.
This dependency causes `lockfile` to be pulled into everywhere that
needs to print out anything. This can be decoupled without too much
trouble by copying in the OSV url. I did so in this PR.
- `config` package references `reporter`, with the above change it also
no longer depends on `lockfile`.
@another-rex another-rex mentioned this issue Aug 30, 2023
Merged
another-rex added a commit that referenced this issue Aug 30, 2023
@another-rex
Revert breaking change in osv.go (#514)
6d34f8a 
Changing the type of Package.Ecosystem would be a breaking change, so
revert that back to string. Doesn't affect too much so looks fine to me,
let's add this to #442 for V2.

Also changed `Response.Vulns` back to type `[]models.Vulnerability`. I
don't think this changes anything since go coerces the types to each
other when setting the value.

And made the not supported error a bit more specific
@github-actions GitHub Actions
Copy link

This issue has not had any activity for 60 days and will be automatically closed in two weeks

@github-actions github-actions bot added the stale The issue or PR is stale and pending automated closure label Jul 22, 2024
@G-Rath G-Rath removed the stale The issue or PR is stale and pending automated closure label Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
V2 Wishlist Enhancements that require a breaking change
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@G-Rath @another-rex