Skip to content

Conversation

@mstrandboge
Copy link
Member

Summary

Continuation of #7733

Checklist

  • No AI generated code was used in this PR

Related issues

resolves #13636

@mstrandboge mstrandboge self-assigned this Feb 13, 2024
@telegraf-tiger telegraf-tiger bot added feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins labels Feb 13, 2024
@mstrandboge mstrandboge marked this pull request as ready for review March 12, 2024 18:38
@mstrandboge mstrandboge assigned powersj and srebhan and unassigned mstrandboge Mar 12, 2024
@srebhan srebhan removed their assignment Mar 14, 2024
@powersj powersj added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Mar 14, 2024
@powersj powersj assigned mstrandboge and srebhan and unassigned powersj and mstrandboge Mar 14, 2024
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

@DStrand1 thanks for the code, just one small question from my side...

Comment on lines +514 to +517
for _, wanted := range wantedFields {
if headerName != wanted.inProc {
continue
}
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be easier to make wantedFields a map and look-up the headerName?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, I don't like the way this code is laid out currently with the field mappings and I plan to refactor it in a follow-up PR. For now I just followed how the other mappings were done, but I can refactor this here instead if you'd prefer

Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Thanks @DStrand1!

@srebhan srebhan merged commit 0f91ca6 into influxdata:master Mar 18, 2024
@github-actions github-actions bot added this to the v1.31.0 milestone Mar 18, 2024
@mstrandboge mstrandboge deleted the fix/13636 branch October 11, 2024 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.

3 participants