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

Ktv/minor improvements #25

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Ktv/minor improvements #25

wants to merge 4 commits into from

Conversation

ktvtrifork
Copy link

based on some debugging related to nil auth state in an app: Thus Added the stacktrace so its possible to backtrack where it happens. (since its likely a user error).
Also minor maintenance (bump) + some cleaning of the errors file and missing tests.

Copy link
Contributor

@kimdv kimdv left a comment

Choose a reason for hiding this comment

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

Har tilføjet nogle kommentar

}
}

public func isKeyLocked() -> Bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Jeg ville bruge computer propperties her i stedet

isKeyServiceError(.keyLocked)
}

public func isWrongPassword() -> Bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Jeg ville bruge computer propperties her i stedet

isKeyServiceError(.badPassword)
}

public func isBiometricFailedError() -> Bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Jeg ville bruge computer propperties her i stedet

///
/// This might be useful for handling unexpected cases from the encryption part of the framework.
/// When the key service fails you don't want to do any drastic fallback, since the server might "just" be down or the user have no internet connection. You will be able to recover later on, from a key service error.
public func isKeyServiceError() -> Bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Jeg ville bruge computer propperties her i stedet

@ktvtrifork
Copy link
Author

Hvad er tanken med breaking changes så? er det "bare" og bump major eller? :)

@kimdv
Copy link
Contributor

kimdv commented Dec 8, 2022

Ah, så det ikke. Venligst ignorer så

@nharbo
Copy link
Contributor

nharbo commented Aug 31, 2023

Er dette noget som skal merges, eller bør dette PR lukkes? :)

@ktvtrifork
Copy link
Author

@nharbo det var nogle ret "nice" forbedringer i forhold til fejlsøgning. jeg husker ikke lige om der var lidt oprydningn også. så jo det vil jeg mene, at det stadig er aktuelt.

@nharbo
Copy link
Contributor

nharbo commented Jun 28, 2024

Skal vi så ikke få gjort noget ved det @ktvtrifork ? :D
Jeg synes i øvrigt Kim havde ret i forhold til de computed properties, men det fungerer fint som funktioner også. Så hvis jeg på nogen måde kan approve, for at få det her snart 2 år gamle PR lukket, så sig til :D

@ktvtrifork
Copy link
Author

@nharbo Ah jeg forklare lige konceptet; computed properties bør kun være konstante ting (kørsels tid) , hvor ting der "kan" tage mere tid (ikke trivielle ting) bør være funktioner. på den måde er det i fx loops nemt og se om du gøre noget der er meget langsomt (eller forkert osv). Ud fra den observation giver det semi mening det er funktioner for det er ikke "trivielt". ellers vil jeg give ret at det læses nok bedre som properties fordi de lyder simple.
Er der andre ting end dem du tænker der skal gøres noget ved?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants