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

Might be a compiler bug or some stuff like that 🤷 #2287

Merged

Conversation

slinkydeveloper
Copy link
Contributor

No description provided.

@AhmedSoliman
Copy link
Contributor

I don't understand. What is the bug or the issue this PR solves?

@slinkydeveloper
Copy link
Contributor Author

The PR name was temporary, I was still testing this out, but it's a funny one :D

So there is an issue with the usage of debug_if_leader! macro within the match, just look at the diff. Without this PR, those log lines will never be printed, as if that code would have never existed. With my fix, those log lines get printed (when debug logging is enabled ofc).

I'm still trying to wrap my head around why this happens though :(

Copy link
Contributor

@tillrohrmann tillrohrmann left a comment

Choose a reason for hiding this comment

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

While I don't fully understand things yet, it seems that I do see some log lines with this PR which I did not see before when running on release. Concretely I was missing the "Send response to ingress with request id" during my testing.

Given that this change shouldn't make things worse by adding some curly braces, I am +1 for merging.

Might be a compiler bug or some stuff like that 🤷
@slinkydeveloper slinkydeveloper merged commit 674501a into restatedev:main Nov 15, 2024
9 checks passed
@slinkydeveloper slinkydeveloper deleted the found-a-compiler-bug!!! branch November 15, 2024 14:36
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