Jwen/redesign profile add ranking #273
Conversation
jiabaow
commented
May 9, 2022
•
jiabaow
commented
|
Code Climate has analyzed commit f04bca4 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (80% is the threshold). This pull request will bring the total coverage in the repository to 82.3% (0.0% change). View more on Code Climate. |
|
|
||
| @Test | ||
| fun clickBackToMain(){ | ||
| val inputProfile = User("Lalisa Bon","bit.ly/3IUnyAF", "[email protected]", 12, 8) |
There was a problem hiding this comment.
It's a test class so shouldn't be an issue, but would it be possible to put links in strings.xml? It would help others with understanding where that link leads to
| "BTS" to 4, | ||
| "Imagine Dragons" to 5, | ||
| "Billie Eilish" to 6) | ||
| val inputProfile = User("Lalisa Bon","bit.ly/3IUnyAF", "[email protected]", |
There was a problem hiding this comment.
Same thing with the link here. It seems to be the same one as the previous comment
| @@ -95,7 +93,7 @@ class ProfileActivityTest { | |||
|
|
|||
| @Test | |||
| fun checkProfileData() { | |||
| val inputProfile = User("Lalisa Bon","bit.ly/3IUnyAF", "[email protected]", 12, 8, 29, 0) | |||
| val inputProfile = User("Lalisa Bon","bit.ly/3IUnyAF", "[email protected]", 12, 8) | |||
There was a problem hiding this comment.
Same here, basically if possible, all the URLS lol
There was a problem hiding this comment.
Good code! As mentioned in some comments, some of the URLs could possibly be put into strings.xml to avoid future confusion as to what those URLs are. If this is not doable, then maybe declaring it as a variable at the top with a small explanatory comment of what the URL is could be useful. If you could do it or if someone else is willing to pick it up, I'll approve right away!
