Updated to app-services-sdk-go in Core SDK #1664
Conversation
jackdelahunt
changed the title
jackdelahunt
force-pushed
the
move-to-new-go-sdk
branch
from
b5be255
to
e5e55ac
Compare
| @@ -42,7 +42,7 @@ require ( | |||
| github.com/prometheus/client_golang v1.14.0 | |||
| github.com/prometheus/client_model v0.3.0 | |||
| github.com/prometheus/common v0.42.0 | |||
| github.com/redhat-developer/app-services-sdk-go/serviceaccounts v0.4.0 | |||
| github.com/redhat-developer/app-services-sdk-core/app-services-sdk-go/serviceaccountmgmt v0.0.0-20230323122535-49460b57cc45 | |||
There was a problem hiding this comment.
Thanks for the PR. Quick question, there is no tag?
There was a problem hiding this comment.
Because we are now publishing all packages in the SDK in one release the tag no longer points to the version (as that is the version of the parent project) but the commit/tag of the latest service account version.
Does that make sense?
There was a problem hiding this comment.
The latest release is https://github.com/redhat-developer/app-services-sdk-core/releases
There was a problem hiding this comment.
I am trying to understand more. Does it mean that the tag 1.0.1 can't be used?
There was a problem hiding this comment.
Yes, but it is pointing to v1.0.1 just the tag is not it
There was a problem hiding this comment.
What I was proposing is not to include the subpackage, but the whole package.
But I see you don't perform tagging at the module level. I wonder if what you are doing is in line with how Go module version management should be performed. The way it is being done users will never be able to use a specific module version via tag.
Maybe what you want are submodules, or not having tags per package (ocm sdk go doesn't do that for example)?
There was a problem hiding this comment.
Yes they won't be able to set a specific tag, I am not sure what you mean by the rest of the comment.
What I was proposing is not to include the subpackage, but the whole package
I think this would be ideal but the rest of teh SDKs are structured like this and this is how the generator generates the code. There may be a way to change this but I will need to look into it. As for including only the top level module, I believe this still wouldn't fix the issue of not being able to specify the tag as the top level module is still a sub directory of the repo
There was a problem hiding this comment.
I think this would be ideal but the rest of teh SDKs are structured like this and this is how the generator generates the code.
What are the "rest of the SDKs"?
There may be a way to change this but I will need to look into it
👍
As for including only the top level module, I believe this still wouldn't fix the issue of not being able to specify the tag as the top level module is still a sub directory of the repo
Not having tags per "subdirectory" would be an approach. ocm sdk go doesn't do that for example, although it contains multiple subdirectories pertaining to different parts of the sdk.
There was a problem hiding this comment.
What are the "rest of the SDKs"?
The TS, Java and Python SDKs
ocm sdk go doesn't do that for example
Yes looking at ocm-sdk-go it just contains subdirectories for each service, this is different to use as they are different modules
There was a problem hiding this comment.
ping @jackdelahunt and @miguelsorianod to see if we reached a conclusion regarding the path forward for this PR.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1664 +/- ##
=======================================
Coverage 82.26% 82.26%
=======================================
Files 161 161
Lines 14989 14989
=======================================
Hits 12330 12330
Misses 2223 2223
Partials 436 436
Flags with carried forward coverage won't be shown. Click here to find out more.
|
jackdelahunt
changed the title
|
@jackdelahunt can you rebase? |
jackdelahunt
force-pushed
the
move-to-new-go-sdk
branch
from
e5e55ac
to
1f18cca
Compare
Done 👍 |
|
ping @jackdelahunt for a rebase |
jackdelahunt
force-pushed
the
move-to-new-go-sdk
branch
from
1f18cca
to
33bc316
Compare
|
@machi1990 done 👍 |
Description
This PR updates the location and version of the
app-services-sdk-goused. The old package used here is now deprecated and is being replaced by the Core SDK.Verification Steps
Checklist (Definition of Done)