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

Adding ApolloAndroidLogger to apollo-android-support. #2824

Merged
merged 5 commits into from Jan 4, 2021

Conversation

AdamMc331
Copy link
Contributor

This introduces a helpful wrapper for the apollo-android-support module that implements the Logger interface from Apollo to output to the logcat.

I've also added this logger to the KotlinSampleApp to log hits and misses to the HttpCache.

Here is a sample screenshot from the logcat:
Screen Shot 2020-12-29 at 3 52 52 PM

* This is an Android wrapper around [Logger] that will take any messages Apollo wants
* to print and log them in the Android logcat.
*/
class ApolloAndroidLogger : Logger {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's worth thinking about what this should mean for Android devs who don't want these logs in production, and whether Apollo wants to be opinionated about this. Perhaps we can provide a solution to turn them off, or just rely on devs to make sure they don't include this logger in production.

If we want to do that, maybe we can add some additional notes inside the documentation here.

Let me know what y'all think!

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we can provide a solution to turn them off, or just rely on devs to make sure they don't include this logger in production.

I'd prefer that decision to be left to the caller. The definition of what "production" is might vary depending the teams/products so I think it's better if Apollo Android doesn't know anything about this. Also, not sending log is as easy as not setting a logger so it should be reasonnably doable at the callsite

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed! So let me know if you think that warrants any additional documentation, but I don't think so - we should rely on the callers to be aware of this concept. :)

README.md Outdated Show resolved Hide resolved
Co-authored-by: Martin Bonnin <martin@mbonnin.net>
@AdamMc331 AdamMc331 changed the title Adding ApolloAndroidLogger to apollo-android-suport. Adding ApolloAndroidLogger to apollo-android-support. Dec 29, 2020
Copy link
Contributor

@martinbonnin martinbonnin left a comment

Choose a reason for hiding this comment

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

Thanks!

@martinbonnin martinbonnin merged commit 9fac3b5 into apollographql:main Jan 4, 2021
martinbonnin added a commit that referenced this pull request Jan 19, 2021
* Adding ApolloAndroidLogger.

* Adding logger to KotlinSampleApp Http Cache.

* Adding note in README.md about apollo-android-support.

* Update README.md

Co-authored-by: Martin Bonnin <martin@mbonnin.net>

* update metalava signatures

Co-authored-by: Martin Bonnin <martin@mbonnin.net>
martinbonnin added a commit that referenced this pull request Jan 20, 2021
* fix binary compatibility (#2812)

* fix binary compatibility

Follow up from #2809

* update metalava signatures

* Ignore Android modules during Gradle sync. (#2811)

This improves autocomplete in IntelliJ/Android Studio

* Better UP-TO-DATE checks (#2817)

* remove the "Build-Timestamp" Manifest property

This makes the builds never up-to-date and compiling things over and
over again

* better up-to-date checks

* Fix broken url in docs (#2829)

## Summary

Easy navigation and easy reading for others

* Fix broken url in docs (#2828)

## Summary

Easy navigation and easy reading for others

* Fix broken url in docs (#2827)

* Fix broken url in docs

## Summary

Easy navigation and easy reading for others

* Update plugin-configuration.mdx

* Adding ApolloAndroidLogger to apollo-android-support.  (#2824)

* Adding ApolloAndroidLogger.

* Adding logger to KotlinSampleApp Http Cache.

* Adding note in README.md about apollo-android-support.

* Update README.md

Co-authored-by: Martin Bonnin <martin@mbonnin.net>

* update metalava signatures

Co-authored-by: Martin Bonnin <martin@mbonnin.net>

* Debugging renovate

* Update dependency gatsby to v2.29.3 (#2836)

Co-authored-by: Renovate Bot <bot@renovateapp.com>

* Update dependency gatsby-theme-apollo-docs to v4.5.12 (#2837)

Co-authored-by: Renovate Bot <bot@renovateapp.com>

* Update the ROADMAP ❄️❄️

* Update get-started-multiplatform.mdx

Multiplatform code is using NSUrlSession, not NSUrlConnection, thanks @dbaroncelli for reporting this!

* bump version to 2.5.0

* release 2.5.0

* version is now 2.5.1-SNAPSHOT

* update release documentation

* build tags

* release 2.5.1

* version is now 2.5.2-SNAPSHOT

* fix worflow file

* release 2.5.2

* version is now 2.5.3-SNAPSHOT

* Update gradle-wrapper.properties (#2852)

* Allow to store Json custom scalars in cache records (#2859)

* Do not sync android modules in the composite project

* Fix storing JsonScalars in Records

* Use Okio's base64 implementation (#2870)

java.util.Base64 is only available on API 26+ on Android while
Okio's implementation works on all API levels.

This fixes #2857

* Fix callback blocks (#2873)

* Fix potential case where the tests would wait forever (#2875)

* configure the Kotlin compiler option for multiplatform projects (#2877)

* Fix broken url in docs (#2829)

## Summary

Easy navigation and easy reading for others

* Fix broken url in docs (#2828)

## Summary

Easy navigation and easy reading for others

* Adding ApolloAndroidLogger to apollo-android-support.  (#2824)

* Adding ApolloAndroidLogger.

* Adding logger to KotlinSampleApp Http Cache.

* Adding note in README.md about apollo-android-support.

* Update README.md

Co-authored-by: Martin Bonnin <martin@mbonnin.net>

* update metalava signatures

Co-authored-by: Martin Bonnin <martin@mbonnin.net>

* Debugging renovate

* Update dependency gatsby to v2.29.3 (#2836)

Co-authored-by: Renovate Bot <bot@renovateapp.com>

* Update dependency gatsby-theme-apollo-docs to v4.5.12 (#2837)

Co-authored-by: Renovate Bot <bot@renovateapp.com>

* Update the ROADMAP ❄️❄️

* Update get-started-multiplatform.mdx

Multiplatform code is using NSUrlSession, not NSUrlConnection, thanks @dbaroncelli for reporting this!

* build tags

* fix worflow file

* Update gradle-wrapper.properties (#2852)

* Allow to store Json custom scalars in cache records (#2859)

* Do not sync android modules in the composite project

* Fix storing JsonScalars in Records

* Fix callback blocks (#2873)

* Fix potential case where the tests would wait forever (#2875)

* configure the Kotlin compiler option for multiplatform projects (#2877)

* cherry-pick of e64d34c

Use okio's base64

* fix integration tests

* fix integration-tests

Co-authored-by: SaintMalik <37118134+saintmalik@users.noreply.github.com>
Co-authored-by: Adam McNeilly <amcneilly331@gmail.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: Nicklas Ansman Giertz <nicklas@ansman.se>
Co-authored-by: emmano <emmanuel.ortiguela@gmail.com>
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.

None yet

2 participants