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

[FEATURE] Use "keyword arguments" for the CloudWatch constructor #82

Open
jdufresne opened this issue Feb 21, 2020 · 2 comments
Open

[FEATURE] Use "keyword arguments" for the CloudWatch constructor #82

jdufresne opened this issue Feb 21, 2020 · 2 comments
Assignees

Comments

@jdufresne
Copy link

Is your feature request related to a problem? Please describe.

I recently wanted to change the $createGroup argument. This is the last argument in a long-ish list of arguments. To solve this, I had to copy and paste all the default arguments when instantiating the handler. It now looks like:

                $handler = new CloudWatch(
                    $client,
                    $log_group,
                    $stream_name,
                    14,            // $retention (default)
                    10000,         // $batchSize (default)
                    [],            // $tags (default)
                    Logger::DEBUG, // $level (default)
                    true,          // $bubble (default)
                    false,         // $createGroup
                );

If the library changes the default value, my instantiation could easily go out of sync.

The values themselves are opaque (what is 14?) requiring me to add multiple inline comments.

Describe the solution you'd like

Allow passing an array of keywords to the constructor. For example:

                $handler = new CloudWatch([
                    'client' => $client,
                    'group' => $log_group,
                    'stream' => $stream_name,
                    'createGroup' => false,
                ]);

This is self documenting and allows omitting default values.

This could be backwards compatible by checking the number and type of arguments and then acting accordingly.

@maxbanton
Copy link
Owner

Hi! Good idea! Can you make a PR for this?

@perfectdeed
Copy link

OK, but what if I want the retention to be infinite?

In the actual code, there is no way to set $retention to null because it has a default >:/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants