Skip to content

Wiki for AutoDispose scope missing error prone check#162

Closed
bangarharshit wants to merge 8 commits intouber:masterfrom
bangarharshit:autodispose-wiki
Closed

Wiki for AutoDispose scope missing error prone check#162
bangarharshit wants to merge 8 commits intouber:masterfrom
bangarharshit:autodispose-wiki

Conversation

@bangarharshit
Copy link
Contributor

Wiki for AutoDispose scope missing error prone check. Fixes issue - #152 and references commit - #156.

Wiki.md Outdated
@@ -0,0 +1,117 @@
## Intro
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please put newlines after all headers. This header should just be the name of the wiki page, and H1 style

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wiki.md Outdated
@@ -0,0 +1,117 @@
## Intro
[Error Prone](https://github.com/google/error-prone) check to detect missing AutoDispose scope within defined scoped elements.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't a complete sentence

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

## Installation

Here are sample configurations which pulls in both the ErrorProne and the AutoDispose check.
### Gradle
Copy link
Collaborator

Choose a reason for hiding this comment

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

Newlines above headers too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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


Here are sample configurations which pulls in both the ErrorProne and the AutoDispose check.
### Gradle
```gradle
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should have an android example here too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Android example is same - annotationProcessor instead of apt

Copy link
Collaborator

Choose a reason for hiding this comment

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

Make both please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wiki.md Outdated
</path>
</annotationProcessorPaths>
<compilerArgs>
<arg>--XepOpt:AutoDisposeLeakCheck=com.bluelinelabs.conductor.Controller,android.app.Activity</arg>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Explain this line with a comment indicating it's for configuration and optional, like in the gradle example

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wiki.md Outdated
^
(see https://github.com/uber/AutoDispose/wiki/Error-Prone-Checker)
```
## Command line flags
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's call this Configuration

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wiki.md Outdated
(see https://github.com/uber/AutoDispose/wiki/Error-Prone-Checker)
```
## Command line flags
By default the checker is applied to standard android components with lifecycle and autodispose interfaces:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Linkify these list items to docs. Also autodispose should be AutoDispose

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which docs are you talking about? Is there a link to javadocs?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wiki.md Outdated
5. ScopeProvider
6. LifecycleOwner

It can be configured by [Error Prone's Command-Line Flags](http://errorprone.info/docs/flags).
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is part of a sentence, so Command-Line Flags all don't need to be capitalized. Error Prone should be Error-Prone

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wiki.md Outdated

It can be configured by [Error Prone's Command-Line Flags](http://errorprone.info/docs/flags).

The following flag is supported and takes input in a form of comma separated list:
Copy link
Collaborator

Choose a reason for hiding this comment

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

comma-separated list of fully qualified class names of Classes with scopes

Also, specify that the flag is UseAutoDispose. I see in the example though that it's AutoDisposeLeakCheck? Is that still the case? If so we should both fix it and also make it more specific, like ClassesWithScope

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wiki.md Outdated

- `-XepOpt:AutoDisposeLeakCheck=com.bluelinelabs.conductor.Controller,android.app.Activity`

The check is now applied to `Controller` and `Activity` only.
Copy link
Collaborator

Choose a reason for hiding this comment

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

"In this case, the check is now...", and emphasize only

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@uber uber deleted a comment Jan 22, 2018
@ZacSweers
Copy link
Collaborator

@harshitbangar123 ping, let's try to minimize how long these PRs are open for

@bangarharshit
Copy link
Contributor Author

Sorry for the delay. I had to travel due to some urgent thing on my end. Working on it now.

Copy link
Contributor Author

@bangarharshit bangarharshit left a comment

Choose a reason for hiding this comment

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

@hzsweers merged multiple formatting issues into 1 commit since they were fairly trivial.

Wiki.md Outdated
@@ -0,0 +1,117 @@
## Intro
[Error Prone](https://github.com/google/error-prone) check to detect missing AutoDispose scope within defined scoped elements.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

## Installation

Here are sample configurations which pulls in both the ErrorProne and the AutoDispose check.
### Gradle
Copy link
Contributor Author

Choose a reason for hiding this comment

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


Here are sample configurations which pulls in both the ErrorProne and the AutoDispose check.
### Gradle
```gradle
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wiki.md Outdated
@@ -0,0 +1,117 @@
## Intro
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wiki.md Outdated
@@ -0,0 +1,117 @@
## Intro
[Error Prone](https://github.com/google/error-prone) check to detect missing AutoDispose scope within defined scoped elements.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wiki.md Outdated
^
(see https://github.com/uber/AutoDispose/wiki/Error-Prone-Checker)
```
## Command line flags
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wiki.md Outdated
(see https://github.com/uber/AutoDispose/wiki/Error-Prone-Checker)
```
## Command line flags
By default the checker is applied to standard android components with lifecycle and autodispose interfaces:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wiki.md Outdated
5. ScopeProvider
6. LifecycleOwner

It can be configured by [Error Prone's Command-Line Flags](http://errorprone.info/docs/flags).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wiki.md Outdated

It can be configured by [Error Prone's Command-Line Flags](http://errorprone.info/docs/flags).

The following flag is supported and takes input in a form of comma separated list:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wiki.md Outdated

- `-XepOpt:AutoDisposeLeakCheck=com.bluelinelabs.conductor.Controller,android.app.Activity`

The check is now applied to `Controller` and `Activity` only.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ZacSweers
Copy link
Collaborator

Let's keep them separate commits in the future still, as I have to basically reread the whole thing to verify now

@@ -1,85 +1,92 @@
## Intro
# Error Prone Checker
Copy link
Collaborator

Choose a reason for hiding this comment

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

Error-Prone

}

plugins {
// we assume you are already using the Java plugin
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just put it anyway

errorprone "com.google.errorprone:error_prone_core:2.1.3"
}

tasks.withType(JavaCompile) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this need to be qualified?

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 works without that also but the flag will be available for all the checks but this is better - 3d7d905.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I meant JavaCompile

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 doesn't need to be. Nullaway is configured like this - https://github.com/uber/NullAway

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

6. LifecycleOwner

It can be configured by [Error Prone's Command-Line Flags](http://errorprone.info/docs/flags).
It can be configured by [Error-Prone's command line flags](http://errorprone.info/docs/flags).
Copy link
Collaborator

Choose a reason for hiding this comment

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

via, not by

Wiki.md Outdated
The following flag is supported and takes input in a form of comma separated list:
The following flag is supported and takes input in a form of comma separated list of fully qualified class names of Classes with scopes:

- `-XepOpt:AutoDisposeLeakCheck=com.bluelinelabs.conductor.Controller,android.app.Activity`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Update for new name

.subscribe(new Consumer<Long>() {
^
(see https://github.com/uber/AutoDispose/wiki/Error-Prone-Checker)
```
Copy link
Collaborator

Choose a reason for hiding this comment

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

Space after code blocks too. This should be human readable before and after rendering

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ZacSweers
Copy link
Collaborator

Updated the wiki with a few little changes. Thanks!

@ZacSweers ZacSweers closed this Feb 3, 2018
@bangarharshit
Copy link
Contributor Author

Thanks for being so patient.

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.

2 participants