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

Add missing cstdint include #39

Merged
merged 1 commit into from
Mar 30, 2023
Merged

Add missing cstdint include #39

merged 1 commit into from
Mar 30, 2023

Conversation

alt-graph
Copy link
Member

This is a bugfix for the missing include of a standard library header as discovered by MSK's Jenkins build. Please review this with priority (but also note that another pull request is still in the queue). :)

[why]
On the openSUSE Tumbleweed Linux distro, compilation of to_number.h fails because of a missing declaration of uint64_t.

[how]
Include cstdint. Also, use the fully namespace-qualified form std::uint64_t as mandated by the standard.

[why]
On the openSUSE Tumbleweed Linux distro, compilation of
to_number.h fails because of a missing declaration of
uint64_t.

[how]
Include cstdint. Also, use the fully namespace-qualified
form std::uint64_t as mandated by the standard.

Signed-off-by: Lars Froehlich <lars.froehlich@desy.de>
@alt-graph alt-graph added the bug Something isn't working label Mar 29, 2023
Copy link
Collaborator

@soerengrunewald soerengrunewald left a comment

Choose a reason for hiding this comment

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

Maybe we should extend our CI with a more recent distro (e.g Ubuntu 22.04, AlmaLinux9, ArchLinux).

@ohensler
Copy link
Collaborator

Looks good to me

@alt-graph
Copy link
Member Author

Maybe we should extend our CI with a more recent distro (e.g Ubuntu 22.04, AlmaLinux9, ArchLinux).

Our Linux tests on Github are already using "Ubuntu-latest", which AFAIK currently translates to 22.04...

@alt-graph alt-graph merged commit 224c57d into main Mar 30, 2023
@alt-graph alt-graph deleted the bugfix/include_cstdint branch March 30, 2023 07:45
@Finii
Copy link
Collaborator

Finii commented Mar 30, 2023

Maybe we should extend our CI with a more recent distro (e.g Ubuntu 22.04, AlmaLinux9, ArchLinux).

Well, we do run on 22.04, no?
https://github.com/actions/runner-images
We specify ubuntu-latest which is indeed not always quite up to date and usually there is a more recent release available by specifying specifically, but not at the moment.

Using anything not 'windows', 'mac-os', 'ubuntu' needs more work, though ;-)

What I did notice that we do not build on Windows; I believe we did that on Gitlab on 'my personal gitlab runner', but obviously that has not been 'ported'. I believe Lars did set up the CI here?

image

@Finii
Copy link
Collaborator

Finii commented Mar 30, 2023

Previously we had windows, see here, running on my 'mini desktop on window sill of old office' 😬

a

For some reason I can not access the runner definitions anymore for GUL on gitlab? 🤔

Ah, I have not been logged in :-D

Here it is

b

@soerengrunewald soerengrunewald mentioned this pull request Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants