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

[pkg/ottl] Added 'Adding new functions/converters' guidelines #33117

Merged
merged 13 commits into from
Jun 14, 2024
22 changes: 22 additions & 0 deletions pkg/ottl/ottlfuncs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1308,3 +1308,25 @@ Functions should be named and formatted according to the following standards.
- Functions that interact with multiple items MUST have plurality in the name. Ex: `truncate_all`, `keep_keys`, `replace_all_matches`.
- Functions that interact with a single item MUST NOT have plurality in the name. If a function would interact with multiple items due to a condition, like `where`, it is still considered singular. Ex: `set`, `delete`, `replace_match`.
- Functions that change a specific target MUST set the target as the first parameter.

## Adding New Functions/Converters
michalpristas marked this conversation as resolved.
Show resolved Hide resolved

Before raising a PR with a new function or converter, raise a PR to verify its acceptance. While acceptance is strongly specific to a specific use case, consider these guidelines for early assessment.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
## Adding New Functions/Converters
Before raising a PR with a new function or converter, raise a PR to verify its acceptance. While acceptance is strongly specific to a specific use case, consider these guidelines for early assessment.
## Adding New Editors/Converters
Before raising a PR with a new Editor or Converter, raise an issue to describe the proposed function. While acceptance strongly depends on factors specific to each proposal, consider these guidelines for early assessment.

TylerHelmuth marked this conversation as resolved.
Show resolved Hide resolved

You're more than welcome to open an issue if:
michalpristas marked this conversation as resolved.
Show resolved Hide resolved
- The proposed functionality is missing,
- The proposed solution significantly improves user experience and readability for very common use cases,
- The proposed solution is more performant in cases where it is possible to achieve the same result with existing options.

It will be up for discussion if your proposal solves an issue that can be achieved in another way but does not improve user experience or performance.

Your proposal likely won't be accepted if:
- User experience is worse and assumes a highly technical user,
- The performance of your proposal very negatively affects the processing pipeline.

As with code, `ottl` should aim for readability first. This means:
michalpristas marked this conversation as resolved.
Show resolved Hide resolved
- Using short, meaningful, and descriptive names,
- Ensuring naming consistency across functions and converters,
michalpristas marked this conversation as resolved.
Show resolved Hide resolved
- Avoiding deep nesting to achieve desired transformations,
- Ensuring functions and converters have a single responsibility,
michalpristas marked this conversation as resolved.
Show resolved Hide resolved
- Following the DRY principle, avoiding multiple functions doing the same thing.
michalpristas marked this conversation as resolved.
Show resolved Hide resolved
Loading