-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[ottl/pkg] Remove the ottlarg struct tag for the arguments and rely on field ordering #25705
[ottl/pkg] Remove the ottlarg struct tag for the arguments and rely on field ordering #25705
Conversation
916cad6
to
3ebc818
Compare
3ebc818
to
fae54c3
Compare
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.
@rnishtala-sumo thanks for handling this. I am going to hold off merging until @evan-bradley has gotten a chance to review next week.
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 like red PRs like this, thanks for taking this on. Could you add an API changelog for this change and update all filterottl and transform processor functions to remove the ottlarg
struct tag as well? Since it isn't a user-facing change for the other components I think it would be easier just to remove the tag in one go.
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
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.
This looks good to me. Tyler and I discussed this offline and we can handle my asks in a follow-up PR.
@TylerHelmuth @evan-bradley thanks for merging this. Apologies I couldn't get to this earlier as I took some time off. |
Remove the ottlarg struct tag and rely on field ordering
Link to tracking Issue: #24874