Support package refactor #336
Conversation
Fiona-Waters
force-pushed
the
support-package-refactor
branch
from
1d5dc2d to
303ddc7
Compare
|
lgtm ! I guess we would need to merge the PR on the common repo first, this way we can test the e2e on GitHub actions here on this pull request before merging this one right? |
Yes that's right. We also need to run |
Fiona-Waters
force-pushed
the
support-package-refactor
branch
4 times, most recently
from
13a11f4 to
85f44cc
Compare
|
/retest |
Fiona-Waters
force-pushed
the
support-package-refactor
branch
6 times, most recently
from
4de6797 to
0a88563
Compare
Fiona-Waters
force-pushed
the
support-package-refactor
branch
from
0a88563 to
6dca388
Compare
Fiona-Waters
force-pushed
the
support-package-refactor
branch
2 times, most recently
from
5603f51 to
08ed2b4
Compare
Fiona-Waters
force-pushed
the
support-package-refactor
branch
from
08ed2b4 to
7e3b4a3
Compare
| @echo " CodeFlareSDKVersion = \"$(CODEFLARE_SDK_VERSION)\"" >> $(DEFAULTS_TEST_FILE) | ||
| @echo " RayVersion = \"$(RAY_VERSION)\"" >> $(DEFAULTS_TEST_FILE) | ||
| @echo " RayImage = \"$(RAY_IMAGE)\"" >> $(DEFAULTS_TEST_FILE) |
There was a problem hiding this comment.
Can you also remove CODEFLARE_SDK_VERSION, RAY_VERSION and RAY_IMAGE references?
AFAIK they are used here, CODEFLARE_SDK_VERSION is also set in
There was a problem hiding this comment.
That brings another point - should the codeflare-common repo be updated for every release (as it reference SDK version)?
IMHO SDK reference is just temporary, to be removed once project-codeflare/codeflare-sdk#292 is implemented.
There was a problem hiding this comment.
Thanks. I have removed those references.
Once that SDK PR is merged we can remove the references to its version in codeflare-common. I can create a follow on issue for this.
The issue is blocked at the moment so I it should probably be updated for any releases in the meantime?
There was a problem hiding this comment.
Follow up issue: project-codeflare/codeflare-common#5
What do we need to do to ensure it is updated in the meantime?
There was a problem hiding this comment.
I am thinking whether it has sense to compose the codeflare-common update into release workflow...
That would be ideal solution but quite a big work considering that it would be removed soon. I would say that the reference can be updated manually in meantime until the SDK tests are implemented.
There was a problem hiding this comment.
Though the MCAD is still referenced in codeflare-common too...
So in the end it may have sense to integrate it in release workflow.
There was a problem hiding this comment.
Ok. Do we need to create an issue or track this anywhere?
There was a problem hiding this comment.
Yes, issue will be needed.
I will create it.
There was a problem hiding this comment.
Created - project-codeflare/codeflare-common#6
Fiona-Waters
force-pushed
the
support-package-refactor
branch
from
4cdbf00 to
868f183
Compare
|
/lgtm |
Fiona-Waters
force-pushed
the
support-package-refactor
branch
from
868f183 to
078ba0f
Compare
Fiona-Waters
force-pushed
the
support-package-refactor
branch
from
078ba0f to
5bb981a
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sutaakar The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Issue link
Closes #237
What changes have been made
The support package has been removed and moved to a separate repo here. Relevant imports have been updated.
Verification steps
Checks