Clarified javadocs in @SharedPrefs with backticks #1878
Conversation
|
Hello @ironjan, thank you for this PR. I have some points:
thanks |
|
okay. ignore the 2nd point. I just realized that we are not adding a value in those javadoc comments. |
|
Thanks for the PR! IMHO, it would be much cleaner, if you would add quotes, and only around string default values. |
|
Thanks for the feedback. I'll add the copyright and adapt this to add quotes only around string default values. |
|
I'm not sure if I chose the correct way to check for StringFields. If the changes look good, I'll rebase this PR. |
|
@ironjan could you please add the update copyright in SharedPrefWithJavaDoc.java`?< 8000 /p> |
|
@ironjan also it would be good to rebase your branch with the latest changes from our develop as it has some changes to the |
39fa28c to
134324c
Compare
|
Copyright notice added and branch rebased. |
There was a problem hiding this comment.
Looks good, can you address my comments? Also please squash the commits into one. No need to add copyright headers in a separate commit.
|
|
||
| if (defaultValueStr != null) { | ||
| fieldMethod.javadoc().append("<p><b>Defaults to</b>: " + defaultValueStr + "</p>\n"); | ||
| boolean isStringPrefField = "stringField".equals(fieldHelperMethodName); |
There was a problem hiding this comment.
Maybe a little bit nicer check would be prefFieldHelperClass == StringSetPrefField.class. So we at least do not rely on magic string constants.
There was a problem hiding this comment.
Shouldnt it be StringPrefField.class instead of StringSetPrefField.class
| fieldMethod.javadoc().append("<p><b>Defaults to</b>: " + defaultValueStr + "</p>\n"); | ||
| boolean isStringPrefField = "stringField".equals(fieldHelperMethodName); | ||
| if (isStringPrefField) { | ||
| fieldMethod.javadoc().append("<p><b>Defaults to</b>: \"" + defaultValueStr + "\"</p>\n"); |
There was a problem hiding this comment.
Can you avoid the code duplication? Extract the only part which is different, to a variable.
For example:
String defaultValueJavaDoc;
if (isStringPrefField) {
defaultValueJavaDoc = "\"" + defaultValueStr + "\"";
} else {
...
}
fieldMethod.javadoc()...| @@ -1,5 +1,6 @@ | |||
| /** | |||
| * Copyright (C) 2010-2016 eBusiness Information, Excilys Group | |||
| * Copyright (C) 2016 the AndroidAnnotations project | |||
There was a problem hiding this comment.
Remove this line, as you did not edited changed file.
Previously, "<p><b>Defaults to</b>: </p>" was generated for string fields with an empty default value. This commit inserts quotes so that the default value in the javadoc is easier to be identified: * "<p><b>Defaults to</b>: \"\"</p>" for empty default strings * "<p><b>Defaults to</b>: \"value\"</p>" for @DefaultString("value")
134324c to
f1df7e5
Compare
|
@WonderCsabo I applied the suggestions and squashed everything together. |
|
Thank you! :) |
I had to wrap this in code tags because the b-tags made the message unreadable