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

cleanup: do not pass EncodingVersion to GenerateVolID() #4498

Merged
merged 1 commit into from
Mar 18, 2024

Conversation

nixpanic
Copy link
Member

The only encoding version that exists is 1. There is no need to have multiple constants for that version across different packages. Because there is only one version, GenerateVolID() does not really require it, and it can use a default version.

If there is a need in the future to support an other encoding version, this can be revisited with a cleaner solution.


Show available bot commands

These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:

  • /retest ci/centos/<job-name>: retest the <job-name> after unrelated
    failure (please report the failure too!)

@nixpanic nixpanic added the ci/skip/e2e skip running e2e CI jobs label Mar 14, 2024
@nixpanic nixpanic requested a review from a team March 14, 2024 09:50
Copy link
Contributor

mergify bot commented Mar 14, 2024

⚠️ The sha of the head commit of this PR conflicts with #4495. Mergify cannot evaluate rules on this PR. ⚠️

@nixpanic
Copy link
Member Author

@Mergifyio refresh

Copy link
Contributor

mergify bot commented Mar 14, 2024

refresh

✅ Pull request refreshed

@nixpanic
Copy link
Member Author

⚠️ The sha of the head commit of this PR conflicts with #4495. Mergify cannot evaluate rules on this PR. ⚠️

I don't know what Mergify means by this. Once other PRs are merged, this can be rebased and hopefully all is fine after that.

@nixpanic
Copy link
Member Author

@Mergifyio rebase

Copy link
Contributor

mergify bot commented Mar 14, 2024

rebase

✅ Branch has been successfully rebased

Madhu-1
Madhu-1 previously approved these changes Mar 14, 2024
internal/util/volid.go Show resolved Hide resolved
@nixpanic nixpanic force-pushed the cleanup/GenerateVolID branch 3 times, most recently from 484b390 to 9d8728c Compare March 14, 2024 14:02
@mergify mergify bot dismissed Madhu-1’s stale review March 14, 2024 14:03

Pull request has been modified.

@nixpanic nixpanic requested review from Madhu-1 and a team March 15, 2024 08:45
@Madhu-1 Madhu-1 removed the ci/skip/e2e skip running e2e CI jobs label Mar 15, 2024
@Madhu-1
Copy link
Collaborator

Madhu-1 commented Mar 15, 2024

Lets not skip the E2E as we have changed the code in go file.

@nixpanic nixpanic requested a review from a team March 15, 2024 11:31
@nixpanic
Copy link
Member Author

@Mergifyio rebase

The only encoding version that exists is `1`. There is no need to have
multiple constants for that version across different packages. Because
there is only one version, `GenerateVolID()` does not really require it,
and it can use a default version.

If there is a need in the future to support an other encoding version,
this can be revisited with a cleaner solution.

Signed-off-by: Niels de Vos <[email protected]>
Copy link
Contributor

mergify bot commented Mar 15, 2024

rebase

✅ Branch has been successfully rebased

@nixpanic nixpanic added the ok-to-test Label to trigger E2E tests label Mar 15, 2024
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.28

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.28

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.28

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.29

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Mar 15, 2024
@mergify mergify bot merged commit 991343d into ceph:devel Mar 18, 2024
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants