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

refactor(tools): update all references to tools.rand #12932

Merged
merged 7 commits into from
Apr 26, 2024

Conversation

chronolaw
Copy link
Contributor

@chronolaw chronolaw commented Apr 25, 2024

Summary

KAG-3143

Checklist

  • The Pull Request has tests
  • A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
  • There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Issue reference

Fix #[issue number]

@pull-request-size pull-request-size bot added size/L and removed size/M labels Apr 25, 2024
@chronolaw chronolaw marked this pull request as ready for review April 25, 2024 07:06
@chronolaw chronolaw requested a review from Water-Melon April 25, 2024 07:06
hanshuebner
hanshuebner previously approved these changes Apr 25, 2024
@hanshuebner
Copy link
Contributor

@bungle expressed concern about this change: Even though we don't like it, we need to consider that our customers use plugins that are forks of plugins that we wrote and that use internal functions, like those from kong.utils. Reorganizational refactorings like provide only small improvements in the grand scheme of things, but they bear substantial risks when customers rely on interfaces that are removed this way.

I think we should rather audit all plugins that we can get our hands on to determine what internal functions they call, add those functions to the PDK and update the plugins to use them from there. Once we've done that, we can do the internal thing.

We could also split up kong.utils, but keep the old interfaces for backwards compatibility.

@kikito

@hanshuebner hanshuebner dismissed their stale review April 25, 2024 14:53

Backward compatibility concerns

@chronolaw
Copy link
Contributor Author

Should we revert this commit? 02aab49

@chronolaw chronolaw changed the title refactor(tools): move rand module out from utils refactor(tools): update all references to tools.rand Apr 26, 2024
@chronolaw
Copy link
Contributor Author

chronolaw commented Apr 26, 2024

After a short discussion, we decided that to keep the utils.* apis for compatibility, @hanshuebner , do you think it is OK now?

Makes sense.

Copy link
Contributor

@Water-Melon Water-Melon left a comment

Choose a reason for hiding this comment

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

LGTM

@hanshuebner hanshuebner merged commit 5dc3b7b into master Apr 26, 2024
25 checks passed
@hanshuebner hanshuebner deleted the refactor/move_rand_out_from_utils branch April 26, 2024 08:00
@github-actions github-actions bot added the incomplete-cherry-pick A cherry-pick was incomplete and needs manual intervention label Apr 26, 2024
@kikito kikito removed the incomplete-cherry-pick A cherry-pick was incomplete and needs manual intervention label Apr 30, 2024
@Kong Kong deleted a comment from team-gateway-bot Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants