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

map: export MapSpec.Compatible #858

Merged
merged 1 commit into from
Nov 23, 2022
Merged

Conversation

olsajiri
Copy link
Contributor

Adding IsCompatibleWith function to export spec.checkCompatibility function.

Suggested-by: Lorenz Bauer lmb@cloudflare.com
Signed-off-by: Jiri Olsa jolsa@kernel.org

@ti-mo
Copy link
Collaborator

ti-mo commented Nov 22, 2022

Ultra-bikeshed:

if m.CompatibleWith(spec) { ... }

or even

if m.Compatible(spec) { ... }

?

@lmb
Copy link
Collaborator

lmb commented Nov 22, 2022

Suggested-by: Lorenz Bauer lmb@cloudflare.com

Please use i@lmb.io, I don't work at CF any more.

@lmb
Copy link
Collaborator

lmb commented Nov 22, 2022

Let me talk to timo, maybe we just make the original function public and leave it at that.

@ti-mo
Copy link
Collaborator

ti-mo commented Nov 23, 2022

I think exporting + renaming MapSpec.checkCompatibility() to MapSpec.Compatible() would make sense.

Exporting MapSpec.checkCompatibility as MapSpec.Compatible

Suggested-by: Lorenz Bauer <i@lmb.io>
Suggested-by: Timo Beckers <timo@isovalent.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
map.go Outdated Show resolved Hide resolved
map.go Outdated Show resolved Hide resolved
@lmb lmb changed the title map: Add IsCompatibleWith function map: export MapSpec.Compatible Nov 23, 2022
@lmb
Copy link
Collaborator

lmb commented Nov 23, 2022

@olsajiri seems like your fork of the library has some changed tags, I get errors when trying to check out your PR via vscode. Maybe you could reset the fork?

@lmb lmb merged commit 79c7e55 into cilium:master Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants