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

Fix ci failure #70

Merged
merged 1 commit into from
Sep 2, 2024
Merged

Fix ci failure #70

merged 1 commit into from
Sep 2, 2024

Conversation

SataQiu
Copy link
Contributor

@SataQiu SataQiu commented Aug 9, 2024

TAG_VERSION and make push-tag was removed by #65
We need to update the CI task as well.

Failure history: https://github.com/volcano-sh/devices/actions/runs/8752334183/job/24019754773

@volcano-sh-bot
Copy link
Collaborator

Welcome @SataQiu!

It looks like this is your first PR to volcano-sh/devices.

Thank you, and welcome to Volcano. 😃

@SataQiu
Copy link
Contributor Author

SataQiu commented Aug 9, 2024

/assign @william-wang

@william-wang
Copy link
Member

Thanks for your contribution. Please take a look at the FOSSA failure.

@SataQiu
Copy link
Contributor Author

SataQiu commented Aug 9, 2024

Hi @william-wang Thank you for your quick feedback.
The CI failure is due to the fact that make push-tag target no longer exists in Makefile, and this PR is the solution for this problem. This fix does not work until the merge.
So it seems like you need to check this PR and merge it manually.

@Monokaix
Copy link
Member

Monokaix commented Aug 9, 2024

Hi @william-wang Thank you for your quick feedback. The CI failure is due to the fact that make push-tag target no longer exists in Makefile, and this PR is the solution for this problem. This fix does not work until the merge. So it seems like you need to check this PR and merge it manually.

From the ci failure message:
image
seems that when checkout code, it didn't checkout to the current PR's commit, so your code is not executed actually.

And the correct checkout action is:
image
which is from https://github.com/volcano-sh/volcano/actions/runs/10284296442/job/28546801702

So I think we should refer to the volcano repo and modify the checkout the action job to solve the ci problem.

@Monokaix
Copy link
Member

Monokaix commented Aug 9, 2024

Maybe we can change

    steps:
      - uses: actions/checkout@v2
to
  - name: Checkout code
    uses: actions/checkout@v3

Signed-off-by: SataQiu <[email protected]>
@SataQiu
Copy link
Contributor Author

SataQiu commented Aug 10, 2024

Good idea, thx @Monokaix
PTAL.

@Monokaix
Copy link
Member

Good idea, thx @Monokaix PTAL.

Seems it still didn't take effect: )

@SataQiu
Copy link
Contributor Author

SataQiu commented Aug 12, 2024

I think it will take effect after merge.
@Monokaix @william-wang

@Monokaix
Copy link
Member

Monokaix commented Sep 2, 2024

Hi @william-wang Thank you for your quick feedback. The CI failure is due to the fact that make push-tag target no longer exists in Makefile, and this PR is the solution for this problem. This fix does not work until the merge. So it seems like you need to check this PR and merge it manually.

As the workflow of github action itself has some problem, so the ci is breakdown, and it's a cycle loop so we should merge it first to solve the ci issus.

@Monokaix
Copy link
Member

Monokaix commented Sep 2, 2024

/lgtm

@Monokaix
Copy link
Member

Monokaix commented Sep 2, 2024

cc @william-wang

@william-wang
Copy link
Member

/approve

@volcano-sh-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: william-wang

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@william-wang william-wang merged commit 09e3c9f into volcano-sh:master Sep 2, 2024
1 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants