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

fix empty string case #1401

Merged
merged 12 commits into from
Apr 25, 2024
Merged

fix empty string case #1401

merged 12 commits into from
Apr 25, 2024

Conversation

fossephate
Copy link
Contributor

Issue Number (if Applicable): Fixes #

Description

Please include a summary of the changes and which issue is fixed / feature is added.

Pull Request - Checklist

  • Initial Manual Tests Passed
  • Double check modified code and verify it with the feature/task requirements
  • Format code
  • Look for code duplication
  • Clear naming for variables and methods

Copy link
Contributor

@OmarHatem28 OmarHatem28 left a comment

Choose a reason for hiding this comment

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

revert the commit that mergs mac-auth branch

@fossephate
Copy link
Contributor Author

Agh I thought the mac auth PR would be merged by now and this was just a quick hack to fix things locally since that branch has the fixes for building on 3.19.5

@fossephate
Copy link
Contributor Author

fossephate commented Apr 23, 2024

Also: I did the same thing for the signing branch, but that one actually has package updates that needs a newer flutter version anyways

@OmarHatem28
Copy link
Contributor

we reverted that since it had some issues, and you fixed some of them later in a separate PR #1398 but still, we are holding on merging it until the issue with biometric auth turned off with restart is fixed

@fossephate
Copy link
Contributor Author

we reverted that since it had some issues, and you fixed some of them later in a separate PR #1398 but still, we are holding on merging it until the issue with biometric auth turned off with restart is fixed

I wasn't able to reproduce the issue more than once so I can't confirm, but I think that issue is unrelated to the changes in the PR (my working theory is that for whatever reason, sometimes secure storage just fails to write), I'll try again to reproduce it tomorrow though and hopefully find a fix

@OmarHatem28
Copy link
Contributor

we reverted that since it had some issues, and you fixed some of them later in a separate PR #1398 but still, we are holding on merging it until the issue with biometric auth turned off with restart is fixed

I wasn't able to reproduce the issue more than once so I can't confirm, but I think that issue is unrelated to the changes in the PR (my working theory is that for whatever reason, sometimes secure storage just fails to write), I'll try again to reproduce it tomorrow though and hopefully find a fix

either way, this is a separate PR, so let's revert the merge commit and merge this

@OmarHatem28 OmarHatem28 merged commit dcfb2b2 into main Apr 25, 2024
1 check passed
@OmarHatem28 OmarHatem28 deleted the path_validation_fix branch April 25, 2024 00:00
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.

2 participants