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

Documentation updates for public developer preview #436

Merged
merged 3 commits into from
Jul 31, 2018

Conversation

eladb
Copy link
Contributor

@eladb eladb commented Jul 30, 2018

  • Remove checked-in theme and install through requirements.txt
  • Modify sidebar to read "AWS Cloud Development Kit"
  • Fix a few bad module links
  • Add installation instructions to "getting started"
  • Remove y-npm and local maven repo information from "getting started"
  • Changed "welcome" to be "index" and embedded a hidden toc there.
  • Added some details about the AWS Construct Library
  • Renamed "reference" to "aws-construct-lib"
  • Renamed "advanced" to "cloudformation"
  • Removed beta reference
  • Updated Welcome page
  • Moved "welcome" to "index".
  • Merged "version reporting" with "tools"

* Remove checked-in theme and install through requirements.txt
* Modify sidebar to read "AWS Cloud Development Kit"
* Fix a few bad module links
* Add installation instructions to "getting started"
* Remove y-npm and local maven repo information from "getting started"
* Changed "welcome" to be "index" and embedded a hidden toc there.
* Added some details about the AWS Construct Library
* Renamed "reference" to "aws-construct-lib"
* Renamed "advanced" to "cloudformation"
* Removed beta reference
* Updated Welcome page
* Moved "welcome" to "index".
* Merged "version reporting" with "tools"
Copy link
Contributor

@rix0rrr rix0rrr left a comment

Choose a reason for hiding this comment

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

I would still recommend calling it Reference. That's what I'd be looking for. Plus, doesn't it include framework docs?

Copy link
Contributor

@RomainMuller RomainMuller left a comment

Choose a reason for hiding this comment

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

Nothing major 👍

modules based on the AWS service the resource belongs to. For example, the
:py:mod:`@aws-cdk/aws-ec2` module includes the `@aws-cdk/aws-ec2.VpcNetwork`
construct which makes it easy to define an `Amazon VPC
<https://aws.amazon.com/vpc>`_ in your CDK app.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the _ there intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's how links are represented in rst apparently...

policy will automatically be modified to allow the specific topic to invoke the
function.

Furthermore, most AWS Constructs expose ``grantXxx`` methods which allow
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like the looks of grantXxx - somehow I think I'll prefer grant~ or grant*. Particularly since there are examples right after. I'm not going to hold a grudge if you disagree, though.

Event-driven APIs
------------------

Many of the AWS constructs include ``onXxx`` methods which can be used to react
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here...

For more information see the :doc:`refs/_aws-cdk_aws-cloudwatch` and :doc:`refs/_aws-cdk_aws-events`
documentation.

Security Groups
Copy link
Contributor

Choose a reason for hiding this comment

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

One thing that bothers me here is that parts of the value of the API (to me) is that people don't have to reason about "Security Groups", because they're scary mythical animals that are had to tamed, and always remain a little savage (so they bite without warning). Anchoring the doc on that name feels like we're asking people to know about the things we'd ideally want to isolate them from.

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 agree in general but we decided that we are going to align to the service semantics, even if the terminology is confusing, in order to improve discoverability and ability of people to map CDK concepts to existing the AWS knowledge base. @rix0rrr and I discussed that we will change the names of the classes to reflect that as well, but for now, I wanted to call out that the "connections programming model" is about security groups.

Added a more explicit ref

project = u'User Guide'
project_desc = u'User Guide'
project = u'AWS Cloud Development Kit'
project_desc = u'AWS Cloud Development Kit'
Copy link
Contributor

Choose a reason for hiding this comment

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

Not liking that project_desc === project.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

Prerequisites
=============

`Node.js (>= 8.11.x) <https://nodejs.org/en/download>`_ - required for the
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have reasons to suggest people use a node that isn't on the 10.0.0 tree? I'm thinking now may be the chance to just update our requirements... And live in the futuristic present?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a good idea to specify the minimum version required. Also 8.11.x is the LTS version of node, which means many people still use it, so we should make sure we at least support that.

Copy link
Contributor

Choose a reason for hiding this comment

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

That makes sense to me. In any case, I do appreciate it being >= 8.11.x and not ^8.11.x.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is :-)


* `AWS Developer blog <https://aws.amazon.com/blogs/developer/>`_
* `GitHub repository <https://github.com/awslabs/aws-cdk>`_

Copy link
Contributor

Choose a reason for hiding this comment

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

This blank line makes the list of things below look like it's incorrectly indented further down; when it isn't. Would remove (or does RST mandate it?)

Elad Ben-Israel added 2 commits July 31, 2018 11:32
* Revert reference to "Reference"
* Some more modifications to "Welcome"
* Added screencast to "Welcome"
@eladb eladb merged commit 4a28a2a into master Jul 31, 2018
@eladb eladb deleted the benisrae/dev-preview-docs branch July 31, 2018 09:09
@NGL321 NGL321 added the contribution/core This is a PR that came from AWS. label Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants