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

chore(logging): fixes and improvement to logging example app #3943

Merged

Conversation

fjnoyp
Copy link
Contributor

@fjnoyp fjnoyp commented Oct 12, 2023

Issue #, if available:

Description of changes:

Fix and clean logger example app.

  1. Initialize Amplify in stateful widget like our other example apps
  2. Fix the navigation error
  3. Remove redundant code by storing category loggers in a map
  4. Fix UI issues with clipping overflow content by refactoring UI elements

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

@fjnoyp fjnoyp requested a review from a team as a code owner October 12, 2023 15:14
@fjnoyp fjnoyp changed the title fix(logging): example app init logic chore(logging): fixes and improvement to logging example app Oct 12, 2023
@fjnoyp fjnoyp force-pushed the feat/logging/cloudwatch-fix-example-app branch from 9a9c89b to 51efa5a Compare October 12, 2023 18:41
this.category,
this.logger,
this.logMsgController, {
this.logLevel = LogLevel.verbose,
Copy link
Member

Choose a reason for hiding this comment

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

nit: better to not set a default value so the log level would be null if not set by user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is that desired behavior though? Since if it's null the logic prevents any log from being sent while wiping the user's log message that they wrote?

Copy link
Member

@NikaHsn NikaHsn Oct 12, 2023

Choose a reason for hiding this comment

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

good observation. the way I see it is if the log level is not set then user would see it and set the value. given the app does not show error if user does not set the log level but put a log message and wipes out the log message it is a good idea to set the log level to vebose.

@haverchuck haverchuck self-requested a review October 12, 2023 22:04
@NikaHsn NikaHsn merged commit b06bfcc into feat/logging/cloudwatch Oct 13, 2023
@Jordan-Nelson Jordan-Nelson deleted the feat/logging/cloudwatch-fix-example-app branch October 20, 2023 18:47
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.

3 participants