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 #142

Merged
merged 9 commits into from
Jun 23, 2018
Merged

Documentation updates #142

merged 9 commits into from
Jun 23, 2018

Conversation

Doug-AWS
Copy link
Contributor

By submitting this pull request, I confirm that my contribution is made under
the terms of the beta license.

This PR contains updates to the environment information (how we get account ID and region),
briefly describes the files that each cdk command creates.

All code examples and output have been checked.

Concepts topic has been refactored into separate sub-topics.

@@ -39,7 +39,7 @@ starts with a pull request to the |cdk|:

git checkout my-branch

* Create a new folder, *aws-cdk-RESOURCE*, in *packages/*,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should remove "creating L2 constructs" from the external docs. L2s will most likely be created by AWS. If we eventually see that we have many external contributors for L2 constructs, we can add a topic on the subject in our CONTRIBUTING guide. But I don't think this belongs to the user manual.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay. I'll pull all the info out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All the L2 construct info was in writing-cdk-apps.rst. I've removed it from the index TOC.

[~] 🛠 Updating MyQueueE6CA6235 (type: AWS::SQS::Queue)
└─ [+] .VisibilityTimeout:
└─ New value: 300
[~] 🛠 Updating CdkBetaQueue3CF282DF (type: AWS::SQS::Queue)
Copy link
Contributor

Choose a reason for hiding this comment

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

@RomainMuller actually fixed this: #136 and it made it to the release

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've updated the doc.


* *bin* contains the following files:

* *hello-cdk.d.ts* is TypeScript source for the entry point to your app
Copy link
Contributor

Choose a reason for hiding this comment

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

.d.ts and .js are not created by the template. They are created by the typescript compiler when we compile the program

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't "cdk init" run tsc? If so, as far as the user is concerned, "cdk init" has created them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about:

bin contains the following files. The command creates hello-cdk.ts from a template,
and the other two files as it runs the TypeScript compiler:

And then I don't say anything about the two generated files; I just list them.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

such as when running **cdk synth**
* *node_module* contains the Node packages you need to develop your TypeScript source
* *package.json* contains metadata for the app.
* *package-lock.json* is the Git lockfile for package.json
Copy link
Contributor

Choose a reason for hiding this comment

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

npm lockfile

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack, I'll fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

* *node_module* contains the Node packages you need to develop your TypeScript source
* *package.json* contains metadata for the app.
* *package-lock.json* is the Git lockfile for package.json
* *README.md* contains a description of a few |toolkit| commands
Copy link
Contributor

Choose a reason for hiding this comment

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

Not only toolkit commands. Just general pointers on how to continue from here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So something like:

contains some information to help you from this point on

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


You can have **jsii** watch for source changes and automatically re-compile those changes using the **watch** option.
You can have **npm** watch for source changes and automatically re-compile those changes using the **watch** option.
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not really npm who is watching. Perhaps just you use the "npm watch" script to watch for ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use npm run watch to automatically ...

Copy link
Contributor

Choose a reason for hiding this comment

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

Yap

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

either express or implied. See the License for the specific language governing permissions and
limitations under the License.

.. _construct_structure:
Copy link
Contributor

Choose a reason for hiding this comment

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

This topic should be just named "Constructs".

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'll change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done (and merged w/construct-overview as just constructs.

will automatically be defined for you.

|l2| members are found in the :py:mod:`@aws-cdk/NAMESPACE` packages,
where RESOURCE is the short name for the associated service,
Copy link
Contributor

Choose a reason for hiding this comment

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

RESOURCE => NAMESPACE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

either express or implied. See the License for the specific language governing permissions and
limitations under the License.

.. _construct_overview:
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't make sense to me that we have two topics:

  • Construct overview
  • Construct structure

Let's merge them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool

@eladb eladb changed the title Dougsch pb1 Documentation updates Jun 20, 2018
@eladb
Copy link
Contributor

eladb commented Jun 20, 2018

Renamed PR title to something a bit more descriptive :-)

@Doug-AWS
Copy link
Contributor Author

Anything else? I'd like to get this distributed to the beta users.

@Doug-AWS Doug-AWS merged commit 433df33 into master Jun 23, 2018
@Doug-AWS Doug-AWS deleted the dougsch-pb1 branch July 2, 2018 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants