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 compatibility layer for v8 inventory #2053

Merged
merged 1 commit into from
Oct 3, 2023
Merged

Conversation

arlimus
Copy link
Member

@arlimus arlimus commented Oct 3, 2023

The kind field was missing to be compatible with the API. Added a layer to translate v9 kind data to the v8 kind enum (which was removed because we no longer have a centralized authority on asset kinds.

@arlimus arlimus changed the title 🐛 add compatibility layer for v8 inventory DRAFT: 🐛 add compatibility layer for v8 inventory Oct 3, 2023
The `kind` field was missing to be compatible with the API. Added a layer to translate v9 kind data to the v8 kind enum (which was removed because we no longer have a centralized authority on asset kinds.

Signed-off-by: Dominik Richter <[email protected]>
@arlimus arlimus changed the title DRAFT: 🐛 add compatibility layer for v8 inventory 🐛 add compatibility layer for v8 inventory Oct 3, 2023
// FIXME: Remove this and solve it at its core
var ids []string
for _, id := range asset.PlatformIds {
if strings.HasPrefix(id, "//") {
Copy link
Member Author

Choose a reason for hiding this comment

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

I noticed a bunch of platform IDs that shouldn't be sent up:

image

Copy link
Member Author

Choose a reason for hiding this comment

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

we need to fix these at their core, this is a sanity-check for v8, but it needs to be removed long-term

Copy link
Member

@chris-rock chris-rock left a comment

Choose a reason for hiding this comment

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

Thank you @arlimus

@chris-rock chris-rock merged commit 1e9dee0 into main Oct 3, 2023
10 checks passed
@chris-rock chris-rock deleted the dom/v8asset branch October 3, 2023 07:18
@github-actions github-actions bot locked and limited conversation to collaborators Oct 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants