Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
This repository was archived by the owner on Feb 26, 2023. It is now read-only.

Add name parameter to @SharedPref#1918

Merged
WonderCsabo merged 2 commits into
androidannotations:developandroidannotations/androidannotations:developfrom
dodgex:1675_add_name_parameter_to_shared_prefCopy head branch name to clipboard
Dec 11, 2016
Merged

Add name parameter to @SharedPref#1918
WonderCsabo merged 2 commits into
androidannotations:developandroidannotations/androidannotations:developfrom
dodgex:1675_add_name_parameter_to_shared_prefCopy head branch name to clipboard

Conversation

@dodgex

@dodgex dodgex commented Dec 11, 2016

Copy link
Copy Markdown
Member

This PR implements the feature request of #1675.

With this PR it is possible to provide a custom name for the preference file of a @SharedPref interface.

@dodgex dodgex requested a review from WonderCsabo December 11, 2016 10:05
@WonderCsabo

Copy link
Copy Markdown
Member

@dodgex please change the PR title, it is pretty ugly how it was generated from the branch name.

@WonderCsabo WonderCsabo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

boolean hasCustomName = !name.trim().isEmpty();
EnumSet<SharedPref.Scope> allowedScopes = EnumSet.of(SharedPref.Scope.ACTIVITY, SharedPref.Scope.UNIQUE);
if (hasCustomName && !allowedScopes.contains(scope)) {
validation.addWarning("SharedPref#name() is only supported for Scope.ACTIVITY and Scope.UNIQUE.");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would add an error() here. Better failing at compile time than spending hours debugging this at runtime.

SharedPref.Scope scope = sharedPrefAnnotation.value();
String name = sharedPrefAnnotation.name();
boolean hasCustomName = !name.trim().isEmpty();
EnumSet<SharedPref.Scope> allowedScopes = EnumSet.of(SharedPref.Scope.ACTIVITY, SharedPref.Scope.UNIQUE);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

:)

@dodgex

dodgex commented Dec 11, 2016

Copy link
Copy Markdown
Member Author

@WonderCsabo I changed from warning to error and now i get a test failure due to a class that has nothing to do with the test?!

java.lang.AssertionError: Expected no errors, found ..\workspace\androidannotations\AndroidAnnotations\androidannotations-core\androidannotations\target\test-classes\org\androidannotations\generation\WindowFeatureAppCompatActivity.java:25: error: package android.support.v7.app does not exist
public class WindowFeatureAppCompatActivity extends android.support.v7.app.AppCompatActivity {

any idea? :(

@WonderCsabo WonderCsabo changed the title 1675 add name parameter to shared pref Add name parameter to @SharedPref Dec 11, 2016
@WonderCsabo

Copy link
Copy Markdown
Member

@dodgex about the test failure: i am just guessing, but maybe ensureOutputDirectoryIsEmpty() deletes something which another test relies on?

@dodgex

dodgex commented Dec 11, 2016

Copy link
Copy Markdown
Member Author

Don't know... i copied that from GenerateJavaDocTest. The error appeas when i add the validation message as error and disappears when i add it as warning...

@dodgex dodgex added this to the 4.3 milestone Dec 11, 2016
@WonderCsabo WonderCsabo merged commit 7b54b2f into androidannotations:develop Dec 11, 2016
@dodgex dodgex deleted the 1675_add_name_parameter_to_shared_pref branch December 11, 2016 13:27
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.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.