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

fix: override the complete configuration of the exporter #1800

Merged

Conversation

yaoyinnan
Copy link
Contributor

Override the complete configuration of the exporter.

Fixes: #1799

Override the complete configuration of the exporter.

Fixes: OpenAtomFoundation#1799

Signed-off-by: yaoyinnan <[email protected]>
@yaoyinnan yaoyinnan requested a review from Mixficsol July 24, 2023 02:40
}
rst = append(rst, info)
}
}

Copy link

Choose a reason for hiding this comment

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

Overall, the code patch looks fine with a few improvements and bug fixes. Here are some suggestions:

  1. Error handling: The code now checks for errors when calling c.InfoAll() and propagates the error if it occurs. This is a good practice, and you should apply the same error handling logic to other functions like Info as well.

  2. Code organization: Currently, the code could benefit from better organization. Group related functions together and ensure readability.

  3. Naming conventions: Follow consistent naming conventions for variables, such as using camelCase instead of snake_case.

  4. Simplification and reducing duplication: Instead of having separate functions for each info section (InfoAll, InfoSubCommand, InfoNoneCommandList, InfoAllCommandList), you can create a single function that takes the command as an argument and returns the result. This would significantly reduce code duplication.

  5. Configuration options: Consider using a struct to hold configuration options instead of separate flags. It would make the code more structured and extensible.

  6. Remove unused code: There are remnants of old code like Select function that are no longer used and can be removed.

By implementing these suggestions, your code will be more maintainable and easier to read.

@AlexStocks AlexStocks merged commit 0737160 into OpenAtomFoundation:unstable Jul 24, 2023
bigdaronlee163 pushed a commit to bigdaronlee163/pika that referenced this pull request Jun 8, 2024
…ndation#1800)

Override the complete configuration of the exporter.

Fixes: OpenAtomFoundation#1799

Signed-off-by: yaoyinnan <[email protected]>
cheniujh pushed a commit to cheniujh/pika that referenced this pull request Sep 24, 2024
…ndation#1800)

Override the complete configuration of the exporter.

Fixes: OpenAtomFoundation#1799

Signed-off-by: yaoyinnan <[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.

pika_exporter: incomplete configuration coverage
3 participants