Skip to content
This repository has been archived by the owner on Jan 26, 2021. It is now read-only.

added CoreData as database , and used only on homepage #156

Closed
wants to merge 1 commit into from
Closed

added CoreData as database , and used only on homepage #156

wants to merge 1 commit into from

Conversation

paraschhugani
Copy link

closes #146
I have added Coredata and only used in the home screen showing the Name of user,,
you may see many files changed because i installed the pods and the newer version of pods was made in-app.
You may ignore that files.

Screen.Recording.2020-12-28.at.1.32.41.PM.mov

@welcome
Copy link

welcome bot commented Dec 28, 2020

Hello there!👋 Welcome to the project!💖

Thank you and congrats🎉 for opening your first pull request.✨ AnitaB.org is an inclusive community, committed to creating a safe and positive environment.🌸Please adhere to our Code of Conduct and Contributing Guidelines.🙌.We will get back to you as soon as we can.😄

Feel free to join us on AnitaB.org Open Source Zulip Community.💖 We have different streams for each active repository for discussions.✨ Hope you have a great time there!😄

Copy link
Member

@vatsalkul vatsalkul left a comment

Choose a reason for hiding this comment

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

Hey @paraschhugani I have looked into the code and I have few queries.

  1. Travis CI build is failing and that is because you have used loadCoreData() in closure without using a explicit self. Please take a look into that
  2. You made 2 loadCoreData function, Why? Can you try to reuse the same function. You can return the optional HomeUserName from loadCoreData function. (It's your choice)
  3. Please remove any commented code which is not useful.
  4. Try not to use print statement. You can use NSLog for important informations.
  5. Ignore pods as well

@@ -32,6 +34,7 @@ class LoginAPI: LoginService {
loginResponseData.message = "Failed to save access token"
}
}
// CoreUserProfile().test(token: String(response.accessToken!))
Copy link
Member

Choose a reason for hiding this comment

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

remove this

@@ -23,6 +23,8 @@ class LoginAPI: LoginService {
.sink { response in
var loginResponseData = LoginModel.LoginResponseData(message: response.message)
// if login successful, store access token in keychain
print("here testting the decdoed respose\(response)")
// CoreUserProfile().saveCore(token: response.accessToken!)
Copy link
Member

Choose a reason for hiding this comment

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

remove this

Copy link
Author

Choose a reason for hiding this comment

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

thanks for review,
I will do the changes,
I have used 2 loadCoredata() function so in future it is easy to customise for different data tables.
can you please say how will I ignore pods, the build is crashing without installing them

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

Successfully merging this pull request may close these issues.

Offline Support using Core Data
2 participants