Skip to content

Conversation

@anderseknert
Copy link
Member

@anderseknert anderseknert commented May 19, 2025

Following up on #7566, and now applying the more exciting modernizations. fmt.Appendf was new to me! But especially the contains checks are so much better IMHO. I have reviewed all changes myself and did a few manual changes where it became obvious that things could be improved a little further.

(the modernize analyzer still has some issues running against OPA, and I have manually worked around those for the time being)

Copy link
Member

@sspaink sspaink left a comment

Choose a reason for hiding this comment

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

Great job! Looks like all good improvements.

Following up on open-policy-agent#7566, and now applying the more exciting
modernizations. fmt.Appendf was new to me! But especially
the contains checks are so much better IMHO. I have reviewed
all changes myself and did a few manual changes where it
became obvious that things could be improved a little further.

(the modernize analyzer still has some issues running against
OPA, and I have manually worked around those for the time being)

Signed-off-by: Anders Eknert <[email protected]>
@anderseknert anderseknert merged commit 8ba08ac into open-policy-agent:main May 20, 2025
28 checks passed
@anderseknert anderseknert deleted the modernize branch May 20, 2025 21:12
aromeyer pushed a commit to aromeyer/opa that referenced this pull request May 21, 2025
Following up on open-policy-agent#7566, and now applying the more exciting
modernizations. fmt.Appendf was new to me! But especially
the contains checks are so much better IMHO. I have reviewed
all changes myself and did a few manual changes where it
became obvious that things could be improved a little further.

(the modernize analyzer still has some issues running against
OPA, and I have manually worked around those for the time being)

Signed-off-by: Anders Eknert <[email protected]>
Signed-off-by: aromeyer <[email protected]>
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