Skip to content

Conversation

@kazakan
Copy link
Contributor

@kazakan kazakan commented Jul 20, 2023

  • Update shared_preferences to 2.2.0.
  • Update shared_preferences_platform_interface to 2.3.0.
  • Implement clearWithParameters and getAllWithParameters
  • Update integration_test.

Copy link
Member

@JSUYA JSUYA left a comment

Choose a reason for hiding this comment

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

Have you tested the newly added API? There is no this api's unit test in shared_preferences's integration_test. (Tested on each platform implementation.)

In this case, it is better to write the results in the comment after testing.

@kazakan
Copy link
Contributor Author

kazakan commented Jul 20, 2023

Have you tested the newly added API? There is no this api's unit test in shared_preferences's integration_test. (Tested on each platform implementation.)

getAllWithParameters and clearWithParameters are used by other methods in original shared_preferences package. It seems they omit test about two api's because if they don't work properly, other methods won't work properly.

@JSUYA
Copy link
Member

JSUYA commented Jul 20, 2023

Have you tested the newly added API? There is no this api's unit test in shared_preferences's integration_test. (Tested on each platform implementation.)

getAllWithParameters and clearWithParameters are used by other methods in original shared_preferences package. It seems they omit test about two api's because if they don't work properly, other methods won't work properly.

However, even before clearWithParameters's failed was fixed, integration_test was still all pass.

@kazakan
Copy link
Contributor Author

kazakan commented Jul 20, 2023

Have you tested the newly added API? There is no this api's unit test in shared_preferences's integration_test. (Tested on each platform implementation.)

getAllWithParameters and clearWithParameters are used by other methods in original shared_preferences package. It seems they omit test about two api's because if they don't work properly, other methods won't work properly.

However, even before clearWithParameters's failed was fixed, integration_test was still all pass.

Yes. I think I was wrong. It seems the test of original package missed this point. For more accurate test, we need to add more test code.

Copy link
Member

@JSUYA JSUYA left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@Swanseo0 Swanseo0 left a comment

Choose a reason for hiding this comment

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

LGTM!

@JSUYA JSUYA merged commit 246d6b0 into flutter-tizen:master Jul 27, 2023
@kazakan kazakan deleted the dev/shared_preferences branch July 30, 2023 14:46
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.

3 participants