Xelink info generation feature #1369
Conversation
There was a problem hiding this comment.
It's a bit disturbing that the earlier gpu_fakedev.go code uses the pattern of:
- define function
- use function (in another function)
But the new code does the opposite.
Could the new functions be ordered so that their functions code is before they are used?
(IMHO that makes the code also marginally easier to review, as you first see / review the function as-is, and then the context in which it is used.)
|
Btw. Why I do not have an option to mark my comments as resolved? Is that reserved only for project maintainers? |
Approved for CI. |
|
commits must be squashed |
|
@hsyrja can you squash? We could then merge. |
|
@hsyrja it's still two commits. |
|
I triggered the checks. If there's no new findings, I think it's fine to squash and merge. |
|
@hsyrja feel free to squash and we can merge. GPU e2e is currently broken. |
cmd/gpu_fakedev/gpu_fakedev.go
Outdated
| func min(val1 int, val2 int) int { | ||
| // TODO replace this with template version of Min(), once Golang standard library gets one | ||
| return int(math.Min(float64(val1), float64(val2))) | ||
| } |
There was a problem hiding this comment.
Latest Go release added generic min() & max() functions:
I.e. this function can be removed when Intel k8s devices project upgrades to go v1.21.
There was a problem hiding this comment.
my thinking is to follow kubernetes, likely after our 0.28 release
|
@hsyrja Ukri's DRA support changes were merged to fakedev code, so your PR will now conflict with the code in the repository, and needs to be rebases & fixed (along with commit squashing requested earlier). |
Looks like this PR is not important so should we just close it? |
This adds xelink info generation option to gpu_fakedev generator.