Skip to content
This repository has been archived by the owner on Apr 16, 2021. It is now read-only.

Added @CheckResult where it matters #43

Merged
merged 1 commit into from
May 30, 2018
Merged

Added @CheckResult where it matters #43

merged 1 commit into from
May 30, 2018

Conversation

oldergod
Copy link
Member

No description provided.

public abstract boolean canStoreSecurely();

/**
* Writes a value to secure storage. Must check {@link #canStoreSecurely()} before subscribing.
*/
@CheckResult
Copy link
Contributor

Choose a reason for hiding this comment

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

for rxjava apis we could use @CheckReturnValue it's an annotation from rxjava

Copy link
Member

Choose a reason for hiding this comment

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

I would avoid bespoke annotations that are unlikely to be supported by most tools.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is for instance supported by Android Lint. Findbugs supports it too if I recall correctly.

Copy link
Member

Choose a reason for hiding this comment

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

The only reason that annotation exists is because RxJava won't take a dependency on something like androidx annotations or error prone annotations. I don't see a reason to promote a random annotation over one shared by tens of thousands of projects.

@mattprecious mattprecious merged commit 8bc7b01 into square:master May 30, 2018
@oldergod oldergod deleted the bquenaudon/check-result branch May 30, 2018 19:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants