Skip to content
This repository was archived by the owner on May 5, 2025. It is now read-only.

Updating Target Docs to Match Template #135

Merged
merged 5 commits into from
Apr 21, 2021

Conversation

alicenstar
Copy link
Contributor

@alicenstar alicenstar commented Apr 21, 2021

Changes

Updated all current Target doc pages to more closely follow the template. Edited capitalization, spelling, and grammatical errors as well as tried to smooth the wording of a few sections, and formatted all pages to be more uniform.

Checklist

  • Go to templates/target/targetExample.md change Prerequisites to Prerequisite(s)
  • Go to templates/target/targetTemplate.md change Prerequisites to Prerequisite(s)
  • Go through all of the targets in the folder targets and change the format to the template

Related issue(s)

Addresses issue #129

Copy link
Contributor

@daceynolan daceynolan left a comment

Choose a reason for hiding this comment

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

Thank you so much for your hard work on this and for taking the initiative to look for grammar and spelling mistakes. I did have a few nitpicky changes. Some of the changes I tried to point out all of the instances, I might have missed some. Please let me know if you have questions or concerns. :)

Some of these are things I just learned pertaining to numbering info and adding bullet points. Please just check to make sure I didn't miss any. Any time we use number lists, we would like it to use all 1. because this will auto-format to correct numbers, and if anything changes in the future the next devs will not need to re number.

Also, any - unordered list should be changed to. *. The do the same in markdown, but we think more files had *


1. AWS API key and secret
- AWS API key and secret
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I just learned this but we can use * here instead of -. Do you mind changing this? Do you also mind changing the template and the example?

Copy link
Contributor

Choose a reason for hiding this comment

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

To be fair, the only reason why I asked not to use - in the previous PR is that we were using * everywhere else, and I didn't want the PR to drift into that discussion and become larger than it should be.

With that being said, I do agree that - would be the more logical choice, since it eliminates the ambiguity between * (bulltet point), * * (italic) and ** ** (bold) <-- I blame markdown for this mess.

I think we should replace * with - in all our lists, but we should do it in one pass, as a separate PR.

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I agree with Antoine in that using - may be easier to differentiate from italic and bold notation. I can make the changes unless there are any strong feelings otherwise.

But I will make the numerical changes and be sure to standardize the notation for unordered lists. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

@antoineco I like using - better *

@thomasgilmore95 is making a rules guideline. Thomas, do you mind adding that all unordered list should use - instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

@antoineco @daceynolan I will make the unordered list to - instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thomasgilmore95 I actually went ahead and changed it already in this PR if that's alright. It'll show up in my next commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

@alicenstar sounds good.

Copy link
Contributor

Choose a reason for hiding this comment

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

@thomasgilmore95 I think @alicenstar only changed the targets so it is okay if you work on the sources you worked on yesterday :)


![AWS Kinesis Target form](../images/aws-targets/aws-kinesis-bridge-create-2.png)

After clicking the `Save` button, the console will self-navigate to the Bridge editor. Proceed by adding the remaining components to the Bridge.

![Bridge overview](../images/aws-targets/aws-kinesis-bridge-create-3.png)

After submitting the bridge, and allowing some configuration time, a green check mark on the main _Bridges_ page indicates that the bridge with the AWS Kinesis Target was successfully created.
After submitting the Bridge, and allowing for some configuration time, a green check mark on the main _Bridges_ page indicates that the Bridge with the AWS Kinesis Target was successfully created.
Copy link
Contributor

Choose a reason for hiding this comment

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

@odacremolbap and @antoineco I like how @alicenstar added a capital letter for Bridge. I assume this was to stand out to the user. Maybe we could highlight them. What do you all think?

Copy link
Member

Choose a reason for hiding this comment

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

I'm also +1 on that.

Copy link
Contributor

@antoineco antoineco Apr 21, 2021

Choose a reason for hiding this comment

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

Agree, since "bridge" is a common word, the capital letter makes it stand out as "TriggerMesh Bridge" and not just a bridge in the common meaning. I like it.

Copy link
Contributor

@daceynolan daceynolan Apr 21, 2021

Choose a reason for hiding this comment

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

Awesome. In the rules guide @thomasgilmore95 is making, Thomas is also going to add this in the rules.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Glad you guys liked that - I figured it would be helpful since your docs already highlight other pertinent words such as Target.


1. AWS API key and secret
- AWS API key and secret
Copy link
Contributor

Choose a reason for hiding this comment

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

These can be turned to *

Copy link
Contributor

@antoineco antoineco Apr 21, 2021

Choose a reason for hiding this comment

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

If we agree that we will use - in the future and address lists which are already unordered in a separate PR, I think those changes can stay.

I'm not feeling strongly about it, just want to avoid discouraging people for something we may replace anyway 🙂

If we decide not to use -, then it's a different story of course.

Copy link
Contributor

Choose a reason for hiding this comment

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

The consensus is to use -


1. ARN for the S3 bucket to store the event
- ARN for the S3 bucket to store the event
Copy link
Contributor

Choose a reason for hiding this comment

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

this could be *

Copy link
Contributor

Choose a reason for hiding this comment

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

Alicen, please ignore these comments.


A Tekton Task must exist prior to creating the Target.
- A Tekton Task must exist prior to creating the Target
Copy link
Contributor

Choose a reason for hiding this comment

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

I would just write Tekton Task


1. Enable the [HTTP Event Collector][hec] data input in the Splunk installation.
1. Create a token for receiving data over HTTP.
2. Create a token for receiving data over HTTP.
Copy link
Contributor

Choose a reason for hiding this comment

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

I just learned this yesterday. If we number lists all as 1. like this previously was, it will auto number.

Copy link
Contributor

@antoineco antoineco left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this! 🙌

Copy link
Contributor

@daceynolan daceynolan left a comment

Choose a reason for hiding this comment

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

Awesome work! Thank you so much for doing this. @odacremolbap do you have anything else?

@daceynolan daceynolan merged commit 5ac2a1c into triggermesh:master Apr 21, 2021
thomasgilmore referenced this pull request in thomasgilmore/docs Apr 21, 2021
* docs: Edit target template and example

* docs: Update and format existings files

* docs: Update and format existings files

* docs: Update and format existing files

* docs: Update and format existing files
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants