Skip to content

add v1 api version#81

Open
lucming wants to merge 1 commit into
volcano-sh:masterfrom
lucming:upgrade_api_version
Open

add v1 api version#81
lucming wants to merge 1 commit into
volcano-sh:masterfrom
lucming:upgrade_api_version

Conversation

@lucming

@lucming lucming commented Jul 11, 2022

Copy link
Copy Markdown

@volcano-sh-bot volcano-sh-bot requested review from k82cn and wpeng102 July 11, 2022 06:48
@volcano-sh-bot

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign wpeng102
You can assign the PR to them by writing /assign @wpeng102 in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details 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

@lucming lucming force-pushed the upgrade_api_version branch from 70a302d to d7d3b1e Compare July 11, 2022 06:49
@Thor-wl Thor-wl requested review from Thor-wl, hwdef and shinytang6 and removed request for k82cn July 11, 2022 07:01
@Thor-wl

Thor-wl commented Jul 11, 2022

Copy link
Copy Markdown
Contributor

@lucming Have you combine this PR with the master branch of volcano.sh/volcano and take a fully UT and e2e test locally?

@Thor-wl Thor-wl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It generally LGTM to me. Please mind the compatibility for users who are still making use of low versions.

@hwdef hwdef left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

By the way, don't understand the meaning of bus, should we change it to a more understandable word

Comment thread pkg/apis/batch/v1/doc.go Outdated
@@ -0,0 +1,21 @@
/*
Copyright 2021 The Volcano Authors.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
Copyright 2021 The Volcano Authors.
Copyright 2022 The Volcano Authors.

Comment thread pkg/apis/batch/v1/labels.go Outdated
@@ -0,0 +1,46 @@
/*
Copyright 2021 The Volcano Authors.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
Copyright 2021 The Volcano Authors.
Copyright 2022 The Volcano Authors.

@lucming

lucming commented Jul 12, 2022

Copy link
Copy Markdown
Author

@lucming Have you combine this PR with the master branch of volcano.sh/volcano and take a fully UT and e2e test locally?

en,volcano.sh/volcano this project changed too much,i want check more,and i will make a pr latter

@lucming

lucming commented Jul 12, 2022

Copy link
Copy Markdown
Author

It generally LGTM to me. Please mind the compatibility for users who are still making use of low versions.

okay,i will pay more attention to it,thanks.

@lucming

lucming commented Jul 12, 2022

Copy link
Copy Markdown
Author

By the way, don't understand the meaning of bus, should we change it to a more understandable word

I'm also very confused about this,can you tell me what he wants to say?

@hwdef

hwdef commented Jul 12, 2022

Copy link
Copy Markdown
Member

By the way, don't understand the meaning of bus, should we change it to a more understandable word

I'm also very confused about this,can you tell me what he wants to say?

I also don't know what this means, may need to consult other more senior maintainers

@lucming lucming force-pushed the upgrade_api_version branch 3 times, most recently from 844631e to fdd33d9 Compare July 12, 2022 09:02
@lucming

lucming commented Jul 12, 2022

Copy link
Copy Markdown
Author

i find https://github.com/volcano-sh/apis/blob/master/pkg/apis/helpers/helpers.go this file in this project,most of them are ways to operate basic resource in k8s,it seems more reasonable to move this file to volcan.sh/volcano

@lucming lucming changed the title upgrade api version add v1 api version Jul 13, 2022
@william-wang

Copy link
Copy Markdown
Member

/hold

@william-wang

Copy link
Copy Markdown
Member

@lucming, It's great to have this pr to upgrade api version. Thank you so much.
We want to see the PR of v1 support in volcano repo prior to merging the api pr. Would you provide modification in volcano repo?

@lucming

lucming commented Jul 19, 2022

Copy link
Copy Markdown
Author

@lucming, It's great to have this pr to upgrade api version. Thank you so much. We want to see the PR of v1 support in volcano repo prior to merging the api pr. Would you provide modification in volcano repo?

I have written a version of the code to upgrade the volcano api version, but there are other things were delayed so have not mentioned pr.

Signed-off-by: lucming <2876757716@qq.com>
@volcano-sh-bot

Copy link
Copy Markdown
Collaborator

@lucming: PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

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.

5 participants