Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean memory leaks #364

Merged
merged 6 commits into from
Dec 30, 2016
Merged

Clean memory leaks #364

merged 6 commits into from
Dec 30, 2016

Conversation

hzalaz
Copy link
Member

@hzalaz hzalaz commented Dec 28, 2016

No description provided.

@hzalaz hzalaz added this to the 2.0.0-rc.2 milestone Dec 28, 2016
@hzalaz hzalaz mentioned this pull request Dec 28, 2016
@cocojoe
Copy link
Member

cocojoe commented Dec 29, 2016

I had a quick run through, there are retain issues in/related to Sign Up
Switch to Sign Up then close Lock.

Quick way to test vs profiler is with a deinit to DatabasePresenter.swift

deinit {
  print("dealloc DatabasePresenter")
}

Also check after inputting in fields.
Also check against my original fix branch for reference.

Copy link
Member

@cocojoe cocojoe left a comment

Choose a reason for hiding this comment

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

Profiles with no leaks now.

@@ -129,22 +129,22 @@ class DatabaseOnlyView: UIView, DatabaseView {

passwordPolicyView.isHidden = true
form.passwordField.errorLabel?.removeFromSuperview()
form.passwordField.onBeginEditing = { _ in
passwordPolicyView.isHidden = false
form.passwordField.onBeginEditing = { [weak passwordPolicyView] _ in
Copy link
Member

Choose a reason for hiding this comment

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

These are not necessary.

@hzalaz hzalaz merged commit 6fce98c into v2 Dec 30, 2016
@hzalaz hzalaz deleted the rework-presenter-due-to-leaks branch December 30, 2016 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants