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

Avoid using pointers to temporary copies of desired extensions. #247

Merged
merged 1 commit into from
Sep 19, 2021

Conversation

ItsBasi
Copy link
Contributor

@ItsBasi ItsBasi commented Sep 16, 2021

validExtensions now references data that remains constant for its entire life-time.

validExtensions now references data that remains constant for its entire life-time.

Signed-off-by: ItsBasi <5033630+ItsBasi@users.noreply.github.com>
@axsaucedo
Copy link
Member

Thank you @ItsBasi for the contribution, looks good. Were you able to test it locally? Currently the unit tests that run with extensions are not part of the Ci due to CPU runner limitations, so you'd need to run the tests locally to validate

@ItsBasi
Copy link
Contributor Author

ItsBasi commented Sep 19, 2021

All 51 test's passed. Although i did not see that any of the run test actually used extensions, so the issue never does surface.

test log.txt

@axsaucedo
Copy link
Member

@ItsBasi you are totally right, it seems I was confusing this example which is not currently in the tests https://kompute.cc/overview/advanced-examples.html#add-extensions

Ok given that all tests pass we can merge, and we can look at adding a test that validates a correct and incorrect extensions (which can work on the CI) - is taht something that you could also be interested in exploring to add?

@axsaucedo axsaucedo merged commit c4b3e6e into KomputeProject:master Sep 19, 2021
@ItsBasi ItsBasi deleted the patch-1 branch September 19, 2021 20:20
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.

2 participants