Closed rohitramkumar308 closed 7 years ago
Thanks for the patch. Here are some suggestions:
I'm not sure if this solves both crashes reported here.
In NoteActivity:+113 you have wrapped almost whole onCreate
function inside try-catch handler. But I think the culprit is just one line where we create a NoteLoader task. (line 145). Since that task is async, I'm not sure if wrapping it under handler would actually help. Overall, I think a better strategy would be to check if we have valid password at beginning of starting an activity (both Note and Sealnote), rather than waiting for something bad to happen. You can just check if its not "null" because anything else must be right, otherwise it wouldn't have been set anyway. Perhaps there is some helper function already somewhere.
You have changed the indentation at some places (mostly alignment of field declarations). There might be cases where we are not consistent. But in general we follow the style we originally have. Also, any indentation changes to lines not related to this patch should goto other patch. This helps reviewers to focus on the code relevant to issue.
The commit message copies the text from the issue. We don't have any strict guideline, but typically like all information formatted in this way whenever possible (refer: git-book) In general, it would be nice to get an explanation of what was wrong and how your fix solves the problem, so that we can review your code quickly. Since the issue itself is vague, this description becomes more important.
Short (50 chars or less) summary of changes
More detailed explanatory text, if necessary. Wrap it to
about 72 characters or so. In some contexts, the first
line is treated as the subject of an email and the rest of
the text as the body. The blank line separating the
summary from the body is critical (unless you omit the body
entirely); tools like rebase can get confused if you run
the two together.
Further paragraphs come after blank lines.
- Bullet points are okay, too
- Typically a hyphen or asterisk is used for the bullet,
preceded by a single space, with blank lines in
between, but conventions vary here
The null check made to Note.java fixes the second crash. (sorry for the auto indent, it is causing a lot of confusion).
Can you share your code style so that it is consistent.
I will make all the other changes. Thanks.
Issue #25 Fix: Bunch of reports with this stacktrace. Might be happening because we don't persist password, and during wakeup its gone from memory.
Replicating: Manually revoke permissions while in the Application.