Skip to content

Conversation

@pankajagrawal16
Copy link
Contributor

@pankajagrawal16 pankajagrawal16 commented Mar 1, 2021

Issue #, if available:

Description of changes:

Preview available here https://pankajagrawal16.github.io/aws-lambda-powertools-java/

Checklist

Breaking change checklist

RFC issue #:

  • Migration process documented
  • Implement warnings (if it can live side by side)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@pankajagrawal16 pankajagrawal16 force-pushed the docs-migration branch 2 times, most recently from 0d58175 to 3e56f2b Compare March 1, 2021 20:19
@msailes
Copy link
Contributor

msailes commented Mar 2, 2021

I don't think I can review this, bit out of my knowledge range. @heitorlessa maybe?

@heitorlessa
Copy link
Contributor

heitorlessa commented Mar 2, 2021 via email

@pankajagrawal16
Copy link
Contributor Author

Yeah I already added preview link in the description

@pankajagrawal16
Copy link
Contributor Author

@pankajagrawal16 pankajagrawal16 requested a review from am29d March 2, 2021 12:06
Copy link
Contributor

@am29d am29d left a comment

Choose a reason for hiding this comment

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

There is one general thought I had when going through the docs. Many code examples don't contain the import statements. While we don't have to bloat the import section of the examples, it is helpful to mention at least the imports from the powertools library. Sometimes IDE can't autoimport, sometimes the class names might be similar within other packages. I had this experience few times when reading other docs and was frustrated to understand where specific classes are coming from.

@pankajagrawal16
Copy link
Contributor Author

@am29d Thanks for great suggestions. I believe I have all of them in place now. Give it another shot. Live preview is updated as well https://me.pankaagr.cloud/aws-lambda-powertools-java/

Copy link
Contributor

@am29d am29d left a comment

Choose a reason for hiding this comment

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

LGTM

@pankajagrawal16 pankajagrawal16 merged commit 161d2ab into master Mar 5, 2021
@pankajagrawal16 pankajagrawal16 deleted the docs-migration branch March 5, 2021 06:41
This was referenced Mar 8, 2021
This was referenced Mar 8, 2021
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.

5 participants