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

fix(pacmak): go docs overview displays repeating description #4081

Merged
merged 4 commits into from
May 9, 2023

Conversation

vinayak-kukreja
Copy link
Contributor

Our go docs have been broken since jsii v1.65.0. This was due to moving to emitting one file per type and headers being added to all those files: Impacting PR

The first occurrence was in docs for cdk v2.39.0 and is still present in the latest version.

The header being added is Version 2 of the AWS Cloud Development Kit library to multiple files. It's determining this value from the package.json file of aws-cdk-lib. I see adding this is most relevant for maven, hence removing it from go since I do not believe it's bringing much value to our documentation. Let me know if you feel otherwise :)


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

@mergify mergify bot added the contribution/core This is a PR that came from AWS. label May 4, 2023
Comment on lines 472 to 478
protected emitHeader(code: CodeMaker) {
if (this.assembly.description !== '') {
code.line(`// ${this.assembly.description}`);
}
code.line(`package ${this.packageName}`);
code.line();
}
Copy link
Contributor

@RomainMuller RomainMuller May 5, 2023

Choose a reason for hiding this comment

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

I think this should still be emitted.... but only in a single file (for example, main.go) so it's not more-than-duplicated...

I think this also has value in IDE integrations, etc... the docsite experience isn't the only place where this might show.

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 see, will make the update. Thank you.

@vinayak-kukreja vinayak-kukreja marked this pull request as ready for review May 8, 2023 19:57
Signed-off-by: Vinayak Kukreja <vinakuk@amazon.com>
Signed-off-by: Vinayak Kukreja <vinakuk@amazon.com>
Signed-off-by: Vinayak Kukreja <vinakuk@amazon.com>
@mergify
Copy link
Contributor

mergify bot commented May 9, 2023

Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it!

@mergify mergify bot added the pr/ready-to-merge This PR is ready to be merged. label May 9, 2023
@mergify
Copy link
Contributor

mergify bot commented May 9, 2023

Merging (with squash)...

@mergify
Copy link
Contributor

mergify bot commented May 9, 2023

Merging (with squash)...

@mergify mergify bot merged commit 4fd596a into main May 9, 2023
27 checks passed
@mergify mergify bot deleted the vkukreja/fix-go-docs branch May 9, 2023 14:07
@mergify mergify bot removed the pr/ready-to-merge This PR is ready to be merged. label May 9, 2023
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.

None yet

2 participants