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

Remove separate packages for platform interface implementation #771

Merged
merged 5 commits into from
Dec 7, 2021

Conversation

natalieparellano
Copy link
Member

Despite having advocated the current approach, I now regret it. Having to duplicate the entire previous platform when a new version is supported is not optimal. E.g. for platform 0.8, the only difference from 0.7 is that standardized sBOM is now supported, but we had to copy the entire directory and essentially do a find and replace to get everything to compile.

It would be awesome if we could still move logic into the platform package (e.g. here, this could be a SupportsSBOM field or method on Platform). But the proposed organization would allow us to do this at our own pace, and reduce a fair amount of duplication.

Signed-off-by: Natalie Arellano <narellano@vmware.com>
@natalieparellano natalieparellano requested a review from a team as a code owner November 29, 2021 22:38
@natalieparellano
Copy link
Member Author

Also makes the code slightly more efficient by having the Platform hold on to a *api.Version instead of a string.

builder_test.go Outdated Show resolved Hide resolved
builder_test.go Outdated Show resolved Hide resolved
Signed-off-by: Natalie Arellano <narellano@vmware.com>
Signed-off-by: Natalie Arellano <narellano@vmware.com>
@codecov
Copy link

codecov bot commented Nov 30, 2021

Codecov Report

Merging #771 (9bec743) into main (071939d) will increase coverage by 3.17%.
The diff coverage is 44.69%.

❗ Current head 9bec743 differs from pull request most recent head 9d710f4. Consider uploading reports for the commit 9d710f4 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #771      +/-   ##
==========================================
+ Coverage   62.32%   65.49%   +3.17%     
==========================================
  Files          59       55       -4     
  Lines        4100     4059      -41     
==========================================
+ Hits         2555     2658     +103     
+ Misses       1283     1121     -162     
- Partials      262      280      +18     
Flag Coverage Δ
os_windows 65.49% <44.69%> (+3.17%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Signed-off-by: Natalie Arellano <narellano@vmware.com>
Signed-off-by: Natalie Arellano <narellano@vmware.com>
@natalieparellano natalieparellano merged commit 42be2d1 into main Dec 7, 2021
@natalieparellano natalieparellano deleted the platform-interface branch December 7, 2021 15:17
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