elastic / elastic-serverless-forwarder

Elastic Serverless Forwarder
Other
35 stars 34 forks source link

Edit ESF docs added by PR #588 #610

Closed dedemorton closed 6 months ago

dedemorton commented 6 months ago

Edits the documentation changes added in #588.

Note that I've tried to make some of the sentences more concise and flow better, but might have inadvertently changed the meaning because the meaning was unclear. Please point out any problems that you find.

I have a few questions for reviewers:

What does this PR do?

Ensure that new and changed content adheres to Elastic style, follows English grammar rules, and is easy to understand.

Why is it important?

Good docs make for happy users.

Checklist

How to test this PR locally

n/a

Related issues

github-actions[bot] commented 6 months ago

File Coverage Lines Branches Missing
All files 100% 100% 100% :white_check_mark:
main_aws.py 100% 100% 100% :white_check_mark:
handlers/__init__.py 100% 100% 100% :white_check_mark:
handlers/aws/__init__.py 100% 100% 100% :white_check_mark:
handlers/aws/cloudwatch_logs_trigger.py 100% 100% 100% :white_check_mark:
handlers/aws/exceptions.py 100% 100% 100% :white_check_mark:
handlers/aws/handler.py 100% 100% 100% :white_check_mark:
handlers/aws/kinesis_trigger.py 100% 100% 100% :white_check_mark:
handlers/aws/replay_trigger.py 100% 100% 100% :white_check_mark:
handlers/aws/s3_sqs_trigger.py 100% 100% 100% :white_check_mark:
handlers/aws/sqs_trigger.py 100% 100% 100% :white_check_mark:
handlers/aws/utils.py 100% 100% 100% :white_check_mark:
share/__init__.py 100% 100% 100% :white_check_mark:
share/config.py 100% 100% 100% :white_check_mark:
share/environment.py 100% 100% 100% :white_check_mark:
share/events.py 100% 100% 100% :white_check_mark:
share/expand_event_list_from_field.py 100% 100% 100% :white_check_mark:
share/factory.py 100% 100% 100% :white_check_mark:
share/include_exlude.py 100% 100% 100% :white_check_mark:
share/json.py 100% 100% 100% :white_check_mark:
share/logger.py 100% 100% 100% :white_check_mark:
share/multiline.py 100% 100% 100% :white_check_mark:
share/secretsmanager.py 100% 100% 100% :white_check_mark:
share/utils.py 100% 100% 100% :white_check_mark:
share/version.py 100% 100% 100% :white_check_mark:
shippers/__init__.py 100% 100% 100% :white_check_mark:
shippers/composite.py 100% 100% 100% :white_check_mark:
shippers/es.py 100% 100% 100% :white_check_mark:
shippers/factory.py 100% 100% 100% :white_check_mark:
shippers/logstash.py 100% 100% 100% :white_check_mark:
shippers/shipper.py 100% 100% 100% :white_check_mark:
storage/__init__.py 100% 100% 100% :white_check_mark:
storage/decorator.py 100% 100% 100% :white_check_mark:
storage/factory.py 100% 100% 100% :white_check_mark:
storage/payload.py 100% 100% 100% :white_check_mark:
storage/s3.py 100% 100% 100% :white_check_mark:
storage/storage.py 100% 100% 100% :white_check_mark:

Minimum allowed coverage is 100%

Generated by :monkey: cobertura-action against bcc7d64370675b6bb774ba05231774de8a2b9e8d

aspacca commented 6 months ago

I have a few questions for reviewers:

  • [x] What do we mean by "recurring" in the following sentence: "The Elastic Serverless Forwarder ensures at-least-once delivery of the forwarded messages recurring to the Continuing queue and Replay queue." It feels like the wrong word, but I'm not sure what you want to use instead. Maybe just "sent to the Continuing queue...."?

I'm sorry, I've used what is a false friend related to Italian. "recurring" should be something like "using"

  • [x] In this sentence: "The remaining time is dedicated to sending a copy of the original messages that contains the remaining events to the Continuing queue." Does the verb "contains" apply to the noun "copy" or "messages"? I want to make sure the subject/verb agreement is correct.

The messages contain the remaining events, the copy, being a copy of those message, contains the remaining events as well :) You're a in a better position than me to choose what's the correct subject/verb agreement :)

  • [x] Minor: Why are we capitalizing "Continuing queue" and "Replay queue"? Does this follow some kind of industry standard or other convention? Also why are these terms italicized everywhere? Typically we only highlight the first occurrence of a term.

I followed that it seemed to me the convention internal to the ESF docs. I might identified the wrong convention. Please change it as it best, thanks

dedemorton commented 6 months ago

I followed that it seemed to me the convention internal to the ESF docs.

Yup, that's always the right approach for smaller updates like this one. Generally we only italicize the first instance of a new term. We avoid capitalizing terms unless they are product names (though we have quite a few exceptions). Sometimes we bend the rule if we are discussing a 3rd party technology that follows a different style. This definitely falls into the category of "stuff only editors care about" :-).

Edited: In case it's not clear from my comment, I'll take care of the changes. Just wanted to make sure there wasn't a specific reason why we are following this convention. Thanks!