-
-
Notifications
You must be signed in to change notification settings - Fork 677
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
fix: Make context.mode always be GoConfigInfo #4203
Conversation
@fmeum I think this needs help to merge |
I added a reproducer in #4206 and then applied the fix. The history of CI checks leads me to believe that this is a genuine failure caused by this change, I just don't know why... |
Hmm your repro looks pretty similar to what I tried, I wonder why I didn't have success. Sadly I don't have a laptop with me to try it right now, but it seems good. Let's ship it! |
We unfortunately can't ship since the fix legitimately breaks existing tests. I'll look into it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I found the fix
What type of PR is this?
Bug fix
What does this PR do? Why is it needed?
Providers and structs with the same fields don't compare equal
Which issues(s) does this PR fix?
Fixes #4197
I wasn't able to figure out a repro in rules_go, it seems like it may rely on some transitions? Not sure we should block landing on this, it seems like this should be the right fix