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

refactor: move pkg/extensions/search/common/oci_layout.go un… #1325

Merged
merged 1 commit into from
Apr 7, 2023

Conversation

nicoldr
Copy link
Contributor

@nicoldr nicoldr commented Apr 3, 2023

…der pkg/test/

What type of PR is this?

Which issue does this PR fix:

What does this PR do / Why do we need it:

If an issue # is not available please add repro steps and logs showing the issue:

Testing done on this change:

Automation added to e2e:

Will this break upgrades or downgrades?

Does this PR introduce any user-facing change?:


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@nicoldr nicoldr changed the title refactor: Cleanup: move pkg/extensions/search/common/oci_layout.go un… refactor: move pkg/extensions/search/common/oci_layout.go un… Apr 3, 2023
pkg/test/oci_layout.go Outdated Show resolved Hide resolved
pkg/test/oci_layout.go Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Apr 3, 2023

Codecov Report

Merging #1325 (cf3df84) into main (38997be) will increase coverage by 0.09%.
The diff coverage is 91.07%.

@@            Coverage Diff             @@
##             main    #1325      +/-   ##
==========================================
+ Coverage   90.37%   90.47%   +0.09%     
==========================================
  Files          97       97              
  Lines       21336    21142     -194     
==========================================
- Hits        19283    19128     -155     
+ Misses       1539     1504      -35     
+ Partials      514      510       -4     
Impacted Files Coverage Δ
pkg/extensions/search/convert/oci.go 94.82% <ø> (-3.86%) ⬇️
pkg/extensions/search/resolver.go 96.40% <ø> (+1.74%) ⬆️
pkg/test/oci_layout.go 91.18% <90.19%> (ø)
pkg/extensions/sync/utils.go 90.17% <100.00%> (+1.89%) ⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@nicoldr nicoldr force-pushed the cleanupCode branch 2 times, most recently from 4e84293 to 3033e81 Compare April 4, 2023 11:00
andaaron
andaaron previously approved these changes Apr 4, 2023
@andaaron
Copy link
Contributor

andaaron commented Apr 4, 2023

The ZAP scan failure is unrelated.

@nicoldr
Copy link
Contributor Author

nicoldr commented Apr 4, 2023

The ZAP scan failure is unrelated.

Thank you

andaaron
andaaron previously approved these changes Apr 4, 2023
@rchincha
Copy link
Contributor

rchincha commented Apr 4, 2023

@nicoldr why do we need pkg/search/common/ anymore now?

@nicoldr
Copy link
Contributor Author

nicoldr commented Apr 5, 2023

@nicoldr why do we need pkg/search/common/ anymore now?

We have here the logic for tags and labels, also the functions to get all kinds of details contained in these labels and tags

@rchincha
Copy link
Contributor

rchincha commented Apr 6, 2023

@nicoldr why do we need pkg/search/common/ anymore now?

We have here the logic for tags and labels, also the functions to get all kinds of details contained in these labels and tags

"common" with what? typically represents some shared code with other parts of the codebase. If this is confined to search/ alone, would just move it up to under pkg/search/ itself.

@nicoldr
Copy link
Contributor Author

nicoldr commented Apr 6, 2023

@nicoldr why do we need pkg/search/common/ anymore now?

We have here the logic for tags and labels, also the functions to get all kinds of details contained in these labels and tags

"common" with what? typically represents some shared code with other parts of the codebase. If this is confined to search/ alone, would just move it up to under pkg/search/ itself.

Hello, Ram. I tried today the scenario in which we move all the code inside common directly under search, but import problems appeared. I had a meeting with Andrei and a new issue was created, and there you can see propositions to move for all the parts that are under search/common right now. Can you take a look at the issue and see if the suggested changes seem alright to you? New issue: #1342

Since it will be quite a work to do there, that PR would be independent from this one and can be reviewed separately

…est/

Signed-off-by: Nicol Draghici <idraghic@cisco.com>
Copy link
Contributor

@rchincha rchincha left a comment

Choose a reason for hiding this comment

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

lgtm

Ok let this be first part of the changes

@rchincha rchincha merged commit 3510ef0 into project-zot:main Apr 7, 2023
24 of 25 checks passed
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.

None yet

3 participants