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

Do not duplicate call to getenv. #369

Merged
merged 1 commit into from
Mar 19, 2024
Merged

Conversation

ghost
Copy link

@ghost ghost commented Mar 9, 2024

Do not duplicate call to getenv.

As described here: #317 (comment)
we shouldn't call getenv twice.

This commit uses a envval variable to store the value of getenv.

Signed-off-by: thb-sb [email protected]

@ghost ghost marked this pull request as ready for review March 9, 2024 10:05
@ghost ghost requested a review from baentsch as a code owner March 9, 2024 10:05
@ghost ghost self-assigned this Mar 9, 2024
@ghost ghost requested a review from bhess March 9, 2024 10:06
@ghost
Copy link
Author

ghost commented Mar 9, 2024

@ghost ghost mentioned this pull request Mar 9, 2024
@baentsch
Copy link
Member

baentsch commented Mar 9, 2024

mirrors.kernel.org seems broken: https://github.com/open-quantum-safe/oqs-provider/actions/runs/8213783404/job/22465567816?pr=369#step:5:92

Was temporary: Re-run turned CI green.

Copy link
Member

@baentsch baentsch left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks, @thb-sb.

Tagging @feventura asking whether you're OK that we merge this before you rebase and squash #317 -- or whether you prefer this to be done the other way around.

@ghost
Copy link
Author

ghost commented Mar 11, 2024

LGTM, Thanks, @thb-sb.

Tagging @feventura asking whether you're OK that we merge this before you rebase and squash #317 -- or whether you prefer this to be done the other way around.

I think it would be easier to wait for 317 to be merged.

@feventura
Copy link
Contributor

I agree, I also think it would be easier to wait for 317 to be merged.

@ghost ghost force-pushed the pr369 branch 4 times, most recently from c3e61c1 to 5398765 Compare March 19, 2024 10:36
As described here: #317 (comment)
we shouldn't call `getenv` twice.

This commit uses a `envval` variable to store the value of `getenv`.

Signed-off-by: thb-sb <[email protected]>
@ghost
Copy link
Author

ghost commented Mar 19, 2024

@baentsch good to merge? I've just rebased it onto main.

@baentsch baentsch merged commit 298a53f into open-quantum-safe:main Mar 19, 2024
45 checks passed
@baentsch
Copy link
Member

@baentsch good to merge? I've just rebased it onto main.

Just merged. Thanks for the improvement, @thb-sb !

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