Skip to content

feat(cfnspec): add CloudFormation documentation to L1 classes#18101

Merged
mergify[bot] merged 6 commits intomasterfrom
huijbers/l1-cfn-docs
Dec 23, 2021
Merged

feat(cfnspec): add CloudFormation documentation to L1 classes#18101
mergify[bot] merged 6 commits intomasterfrom
huijbers/l1-cfn-docs

Conversation

@rix0rrr
Copy link
Copy Markdown
Contributor

@rix0rrr rix0rrr commented Dec 20, 2021

Include CloudFormation documentation in the generated L1 docstrings.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@rix0rrr
feat(cfnspec): add CloudFormation documentation to L1 classes
72d262f 
Include CloudFormation documentation in the generated L1 docstrings.
@rix0rrr rix0rrr requested a review from a team December 20, 2021 20:18
@rix0rrr rix0rrr self-assigned this Dec 20, 2021
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Dec 20, 2021
@rix0rrr
Copy link
Copy Markdown
Contributor Author

rix0rrr commented Dec 20, 2021

Pre-emptive FAQ:

Q: How are we going to keep the data file up-to-date?

A: I would like to consider that of scope for this PR. Rest assured I have a cunning plan 🤓

@gitpod-io
Copy link
Copy Markdown

gitpod-io bot commented Dec 21, 2021

@rix0rrr rix0rrr added pr-linter/exempt-readme The PR linter will not require README changes pr-linter/exempt-test The PR linter will not require test changes labels Dec 21, 2021
iliapolo
iliapolo reviewed Dec 21, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@iliapolo iliapolo left a comment

Choose a reason for hiding this comment

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

@rix0rrr This is very cool, but...in-spite of your pre-emptive clarification, I am somewhat uncomfortable pushing this without a maintenance mechanism.

Once the docs get stale in significant way, this will result in a degraded experience than what we have now. And we have no way of knowing when this will happen. I think we should hold back until we at least have a manual process or something.

Also, can we push a few tests so we can see a diff when this code changes?

@rix0rrr
Copy link
Copy Markdown
Contributor Author

rix0rrr commented Dec 21, 2021

Here's an example of a generated L1 file: https://pastebin.com/fJRmT3E4

@iliapolo
Copy link
Copy Markdown
Contributor

iliapolo commented Dec 21, 2021

@rix0rrr as we discussed lets currently hold off on this until we have the update mechanism in-place as well. Converting to draft in the meanwhile.

@iliapolo iliapolo marked this pull request as draft December 21, 2021 13:43
@rix0rrr rix0rrr marked this pull request as ready for review December 23, 2021 09:56
@rix0rrr
Copy link
Copy Markdown
Contributor Author

rix0rrr commented Dec 23, 2021

There is an update mechanism in place now

@rix0rrr rix0rrr requested a review from a team December 23, 2021 13:50
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Dec 23, 2021

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Copy Markdown
Collaborator

aws-cdk-automation commented Dec 23, 2021

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: 8646124
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot merged commit 0ed661d into master Dec 23, 2021
@mergify mergify bot deleted the huijbers/l1-cfn-docs branch December 23, 2021 17:55
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Dec 23, 2021

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

iliapolo added a commit that referenced this pull request Dec 25, 2021
@iliapolo
"revert(cfnspec): add CloudFormation documentation to L1 classes (#18101
2924cf3 
)"

This reverts commit 0ed661d.
rix0rrr added a commit that referenced this pull request Dec 27, 2021
@rix0rrr
chore(cfnspec): CloudFormation documentation in L1 classes, again
fb5ccbb 
Re-draft of #18101. The previous implementation rendered invalid
escape sequences which would break the Java rendering.

Fixed in this iteration.
@rix0rrr rix0rrr mentioned this pull request Dec 27, 2021
Merged
mergify bot pushed a commit that referenced this pull request Dec 27, 2021
@rix0rrr
chore(cfnspec): CloudFormation documentation in L1 classes, again (#1…
a15250e 
…8190)

Re-draft of #18101. The previous implementation rendered invalid
escape sequences which would break the Java rendering.

Fixed in this iteration.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this pull request Feb 21, 2022
@rix0rrr @TikiTDO
feat(cfnspec): add CloudFormation documentation to L1 classes (aws#18101
b66d26b 
)

Include CloudFormation documentation in the generated L1 docstrings.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this pull request Feb 21, 2022
@rix0rrr @TikiTDO
chore(cfnspec): CloudFormation documentation in L1 classes, again (aw…
86a4e3c 
…s#18190)

Re-draft of aws#18101. The previous implementation rendered invalid
escape sequences which would break the Java rendering.

Fixed in this iteration.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iliapolo iliapolo iliapolo approved these changes

Assignees

@rix0rrr rix0rrr

Labels

@aws-cdk/cfnspec contribution/core This is a PR that came from AWS. pr-linter/exempt-readme The PR linter will not require README changes pr-linter/exempt-test The PR linter will not require test changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rix0rrr @iliapolo @aws-cdk-automation