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

Add a Cache.deleteStudent(username) method instead of using Cache.setStudent(username, null) #21

Open
abhijitparida opened this issue Oct 23, 2018 · 13 comments
Milestone

Comments

@abhijitparida
Copy link
Owner

abhijitparida commented Oct 23, 2018

File location: app/src/main/java/app/abhijit/iter/data/Cache.java

(update/add unit tests accordingly)

@jashasweejena
Copy link
Contributor

I'd like to take this up!

@abhijitparida
Copy link
Owner Author

Alright! 🎉

@jashasweejena
Copy link
Contributor

Do I need to modify only the implementation in data/Cache.java or do I need to modify the method uses too?

@abhijitparida
Copy link
Owner Author

You also need to change all the method calls.
Bonus: add unit tests for the new method.

@jashasweejena
Copy link
Contributor

You also need to change all the method calls.
Bonus: add unit tests for the new method.

Never added a unit test myself. Could you walk me through briefly, if you don't mind? 😃

@jashasweejena
Copy link
Contributor

Could I work on it at the night? if it's all good with you!

@abhijitparida
Copy link
Owner Author

abhijitparida commented Oct 23, 2018

I fixed the build config issue. Update your fork accordingly. I also recommend using different branches for each pull request. See: https://guides.github.com/introduction/flow/

Never added a unit test myself. Could you walk me through briefly, if you don't mind? 😃

Actually, I haven't set up tests for Cache yet. So you don't have to worry about that.

Here is an example unit test, for Subject: SubjectTest.java

Could I work on it at the night? if it's all good with you!

Take your time!

@jashasweejena
Copy link
Contributor

Actually, I haven't set up tests for Cache yet. So you don't have to worry about that.

So I'd have to add tests for the entire 'Cache.java' if I'm not mistaken?

Here is an example unit test, for Subject: SubjectTest.java

Thank you!

Take your time!

Sure 😁

@abhijitparida
Copy link
Owner Author

No, you don't have to add tests for Cache.

@jashasweejena
Copy link
Contributor

Okay. Thank you!

@jashasweejena
Copy link
Contributor

No, you don't have to add tests for Cache.

So, all in all, I don't need to work with tests?

@abhijitparida
Copy link
Owner Author

No. Just ensure that the app builds successfully without any warnings/errors.

@abhijitparida abhijitparida changed the title Add a Cache.deleteStudent() method instead of using Cache.setStudent(null, null) Add a Cache.deleteStudent(username) method instead of using Cache.setStudent(username, null) Oct 23, 2018
@abhijitparida abhijitparida added this to the v2.7 milestone Oct 24, 2018
@pedroqueiroz
Copy link

Hi!

Is this issue taken by anyone?

Thanks! :)

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

No branches or pull requests

3 participants