Refactored documentation #630
Conversation
There was a problem hiding this comment.
@federicaciuffo Thanks so much for putting this massive PR together. I've got a few minor comments/questions sprinkled throughout but overall this is looking good.
There was a problem hiding this comment.
How much does having this html file give us? I think ideally we'd stick to markdown just for ease of maintenance.
There was a problem hiding this comment.
I used Karpenter docs as reference because it is really well written. Having a proper home gives a professional look and I kept it very clean for easy maintenance.
There was a problem hiding this comment.
I'm curious how you're thinking about this versus, for example, the VPC Lattice User Guide. How much do we repeat here versus link to the user guide?
There was a problem hiding this comment.
This file is just a reviewed version of this already existing file. For these files, we aimed to maintain the concepts and rearrange the information a bit for better clarity. On the other hand, except the SN, S, etc concepts (approx 20 lines) this doc is addressing EKS users and use-cases.
docs/contributing/developer.md Outdated
| E2E tests require a Service Network named `test-gateway` with cluster VPC associated to run. | ||
| You can either set up Service Network manually or use DEFAULT_SERVICE_NETWORK option when running controller locally. (e.g. `DEFAULT_SERVICE_NETWORK=test-gateway make run`) |
There was a problem hiding this comment.
We don't capitalize "service network" elsewhere in our docs, e.g. https://docs.aws.amazon.com/vpc-lattice/latest/ug/how-it-works.html
Should we be doing it here?
There was a problem hiding this comment.
agree, changed in all md files
docs/guides/custom-domain-name.md Outdated
| | ||
| * You MUST have a registered hosted zone (e.g. `my-test.com`) in route53 and complete the `Prerequisites` mentioned in [Configure a custom domain name for your service](https://docs.aws.amazon.com/vpc-lattice/latest/ug/service-custom-domain-name.html#dns-associate-custom). | ||
| * If you are not using ExternalDNS, you should manually associate your custom domain name with your service following [Configure a custom domain name for your service](https://docs.aws.amazon.com/vpc-lattice/latest/ug/service-custom-domain-name.html#dns-associate-custom). | ||
| * You MUST have a registered hosted zone (e.g. `my-test.com`) in Route53 and complete the `Prerequisites` mentioned in [this section](https://docs.aws.amazon.com/vpc-lattice/latest/ug/service-custom-domain-name.html#dns-associate-custom) of the Amazon VPC Lattice documentation. |
There was a problem hiding this comment.
I think the link should just be: ttps://docs.aws.amazon.com/vpc-lattice/latest/ug/service-custom-domain-name.html with no anchor. The "Prerequisites" section is toward the top.
There was a problem hiding this comment.
agree, changed
| - Concepts: | ||
| - Introduction: concepts/concepts.md | ||
| - Concepts: concepts/overview.md | ||
| - User Guides: |
There was a problem hiding this comment.
Should we include a link for guides/multi-cluster.md?
There was a problem hiding this comment.
Deleted the file because it created duplicity, this part is already explained in the walkthrough (multi-cluster part). (and it is not in the current version neither)
| # Build the deploy.yaml | ||
| make build-deploy | ||
| cp "deploy.yaml" "examples/deploy-$RELEASE_VERSION.yaml" | ||
| cp "deploy.yaml" "files/controller-installation/deploy-$RELEASE_VERSION.yaml" |
There was a problem hiding this comment.
Wondering if we can figure out a more descriptive name than "files" here. Should we just move these two directories to top-level? So we have /examples and /controller-installation?
There was a problem hiding this comment.
we could but it’s a lot of work and if I miss a dependency some command will not work. is this a must or a nice to have?
There was a problem hiding this comment.
Probably just a nice to have
2 participants
What type of PR is this? Refactor documentation
Which issue does this PR fix: documentation
What does this PR do / Why do we need it: refactor documentation
If an issue # is not available please add repro steps and logs from aws-gateway-controller showing the issue: NA
Testing done on this change: tested walkthroughs
Automation added to e2e: NA
Will this PR introduce any new dependencies?: changed the
/examplespath to/files/examplesand/files/controller-installation. This needs to be taken into consideration when testing due to some github raw files referred into the documentation (dependencies within the documentation have been updated to new paths).Will this break upgrades or downgrades. Has updating a running cluster been tested?: No
Does this PR introduce any user-facing change?: yes
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.