chor: Improve the code quality of the Registry class #55
Conversation
Signed-off-by: AayushSaini101 <[email protected]>
There was a problem hiding this comment.
Hi,
Thanks for your PR.
But I don't see how it will be an improvement. API usage is much more complicated. Passing some class via new, arrays instead of varagrs etc....
I would prefer to focus more on implementing OCI feature life referral.
There are also tasks related to release and publishing that are more priority so that user can consume the library easily.
I will put this PR on hold.
| @@ -184,7 +184,7 @@ void testShouldPushAndPullMinimalArtifact() throws IOException { | |||
| Files.writeString(file1, "foobar"); | |||
|
|
|||
| // Upload | |||
| Manifest manifest = registry.pushArtifact(containerRef, file1); | |||
| Manifest manifest = registry.pushArtifact(new Registry.PushArtifactBuilder(containerRef, file1)); | |||
There was a problem hiding this comment.
For me it's far from an improvement passing some object via new.
| private String artifactType = null; // Optional parameter | ||
| private Annotations annotations = Annotations.empty(); // Default value | ||
| private Config config = Config.empty(); // Default value | ||
| private Path[] paths; // Required parameter |
There was a problem hiding this comment.
Not hood for user experience
| public Manifest pushArtifact( | ||
| ContainerRef containerRef, String artifactType, Annotations annotations, Path... paths) { | ||
| return pushArtifact(containerRef, artifactType, annotations, Config.empty(), paths); | ||
| public Manifest pushArtifact(PushArtifactBuilder builder) { |
There was a problem hiding this comment.
Shouldn't it be private ?
| private Path[] paths; // Required parameter | ||
|
|
||
| // Constructor for required fields | ||
| public PushArtifactBuilder(ContainerRef containerRef, Path... paths) { |
There was a problem hiding this comment.
Shouldn't it be private?
| } | ||
|
|
||
| // Setter for config | ||
| public PushArtifactBuilder withConfig(Config config) { |
There was a problem hiding this comment.
Shouldn't it be private?
Thanks @jonesbusy In that case, i will start working on the proper roadmap tasks of the oras-java #11 |
|
Also perhaps what we miss is clear API? What we expose as public methods. What is implementation details etc... |
Description
Improve the code quality of the Registry class for the pushArtificatMethod()
Related #50.
Testing done