update readme and add contributing guide #132
Conversation
README.md
Outdated
|
|
||
| The AWS Cloud Development Kit (CDK) is used to perform infrastructure specification, setup, management, and teardown. You can learn more about infrastructure-as-code using the CDK [here](https://docs.aws.amazon.com/cdk/v2/guide/home.html). | ||
|
|
||
| ## Quick Start Guide | ||
| TODO |
There was a problem hiding this comment.
What are the minimal steps to get started?
There was a problem hiding this comment.
- Install the prereqs
- run ./manage_arkime.py cluster-create --name ClusterName (see manage_arkime.py cluster-create --help for important options)
- run ./manage_arkime.py vpc-add --cluster-name ClusterName --vpc-id VPCID # to add the cluster
- run ./manage_arkime.py get-login-details --name ClusterName # to see default login details
| @@ -316,50 +323,6 @@ Finally, you can create an interactive session using the AWS CLI. You'll need t | |||
| aws ecs execute-command --cluster <your cluster ID> --container CaptureContainer --task <your task id> --interactive --command "/bin/bash" | |||
| ``` | |||
|
|
|||
| ## How to run the unit tests & lint | |||
There was a problem hiding this comment.
Moved this to CONTRIBUTING.md
|
In my opinion sections "How to run the AWS All-in-One CLI" through "Performing CDK Bootstrap" should be removed from this readme and included in our website on a specific Arkime AWS AIO page (I can help create this!). I think a readme for a project of this scale should be more of an overview. Then there should be links to documentation on the details. |
README.md
Outdated
| @@ -1,9 +1,15 @@ | |||
| # Arkime AWS All-in-One | |||
|
|
|||
| The goals of this project are 1) provide a demonstration of how Arkime can be deployed in a cloud-native manner and 2) provide scripting to enable users to easily begin capturing the traffic in their existing AWS cloud infrastructure. | |||
| The goals of this project: | |||
| 1) Demonstrate how Arkime can be deployed in a cloud-native manner | |||
There was a problem hiding this comment.
I think we should go down to 1 goal, which should be #2
README.md
Outdated
| ``` | ||
| npx eslint . | ||
| ``` | ||
|
|
||
| ## Performing CDK Bootstrap |
There was a problem hiding this comment.
Should this go at the beginning near the "How to run the AWS All-in-One CLI" section?
|
I don't disagree about moving to a web page, but lets get a README done and then do a second iteration. :) |
|
We should include a list of all the command-line parameters including defaults and descriptions. |
| * Provide a clear and descriptive title | ||
| * Clearly describe the problem and solution | ||
| * Include the relevant issue number(s) if applicable | ||
| * Run unit tests and lint as [described below](how-to-run-the-unit-tests-&-lint) |
There was a problem hiding this comment.
I think we have to have a higher bar, here. There are a lot of subtle ways things could break that are outside the purview of the unit tests. For example - there could be a mismatch between the Python and CDK sides on a passed context variable. In all my own PRs, I post the CLI output from the commands I run to test the changes actually work, and I'd feel more comfortable if that's the bar we maintained for all PRs.
How about:
We require the unit tests and Python/TypeScript linters to pass before merging PRs. Additionally, any substantive changes should include:
* A clear explanation of what automated and manual testing you performed
* The output from the CLI of the test commands you ran to confirm the change operated as expected
* Screenshots of the AWS Console/Arkime Dashboard/etc as appropriate to demonstrate the change operates as expected
In other words - just having passing unit tests and linters doesn't tell us much unless the change is completely trivial.
Thoughts?
License
I confirm that this contribution is made under an Apache 2.0 license and that I have the authority necessary to make this contribution on behalf of its copyright owner.