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

Update literals to named arguments in TopicLessonsFragmentTest.kt #2515

Closed
srikanthkb opened this issue Jan 19, 2021 · 10 comments · Fixed by #2524
Closed

Update literals to named arguments in TopicLessonsFragmentTest.kt #2515

srikanthkb opened this issue Jan 19, 2021 · 10 comments · Fixed by #2524
Assignees
Labels
good first issue This item is good for new contributors to make their pull request. Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@srikanthkb
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Update the literals passed as argument to named arguments. This improves code readability and provides better context to the readers. One such instance is shown as an example below.

Describe the solution you'd like
Update the argument in line 246 below:

atPositionOnView(
R.id.story_summary_recycler_view,
1,
R.id.chapter_recycler_view
)

to the following code:

        atPositionOnView(
          R.id.story_summary_recycler_view,
          position = 1,
          R.id.chapter_recycler_view
        )

Additional context

  1. Identify other instances of such changes in TopicLessonsFragmentTest, update the code with named arguments.
  2. Identify other files/classes where such changes, update with named arguments for better readability.
@srikanthkb srikanthkb added Type: Improvement good first issue This item is good for new contributors to make their pull request. labels Jan 19, 2021
@harshpatel63
Copy link
Contributor

I'll like to work on it. Can you assign me this issue??

@srikanthkb
Copy link
Contributor Author

@Sci3fic Have you completed the onboarding procedures from here? https://github.com/oppia/oppia-android/wiki
Let us know if all of the procedures and setup is complete.

@srikanthkb srikanthkb changed the title Update literals to named arguments to provide better context to readers Update literals to named arguments to provide better context Jan 19, 2021
@harshpatel63
Copy link
Contributor

@Sci3fic Have you completed the onboarding procedures from here? https://github.com/oppia/oppia-android/wiki
Let us know if all of the procedures and setup is complete.

Done with the procedures and setup.

Just for clarification: I need to check each and every Kotlin file for the issue. Right??

@srikanthkb
Copy link
Contributor Author

Yes. Currently, we have few pre-push git hooks, which warns you if there are any linting errors before you push it to your branch. If you see any errors/warnings, the push will fail, and you will be asked to fix the errors.

@anandwana001
Copy link
Contributor

I suggest creating multiple sub issues for this. Tracking in one issue might be complex.

@srikanthkb
Copy link
Contributor Author

I suggest creating multiple sub issues for this. Tracking in one issue might be complex.

Agreed. On what basis should we split these into sub issues or should we create independent good-first-issues for the identified files ?

@anandwana001
Copy link
Contributor

we can create package wise. Are we including test files too? and what all modules we are covering in this?

@srikanthkb
Copy link
Contributor Author

I thought this would be a good first issue, for all of the Tests (Ex: TopicLessonsFragmentTest,AdministratorControlsActivity etc) in app module, specifically related to RecyclerView as it would contain the argument position in the scrollToPosition method.
But if you think, we should pursue this for the entire codebase, then maybe creating independent issues for different modules will make it easier to track the changes. What do you think ?

@anandwana001
Copy link
Contributor

yes, let's create issues package vice.

@srikanthkb srikanthkb changed the title Update literals to named arguments to provide better context Update literals to named arguments in TopicLessonsFragmentTest.kt Jan 21, 2021
@srikanthkb
Copy link
Contributor Author

srikanthkb commented Jan 21, 2021

yes, let's create issues package vice.

Renamed this issue specifically for TopicLessonsFragmentTest. I will create new issues for tracking changes at the package level.

rt4914 pushed a commit that referenced this issue Jan 21, 2021
…tTest.kt. (#2524)

* Updated literals to named argumentsin TopicLessonsFragmentTest.kt.

* Update TopicLessonsFragmentTest.kt

* Update TopicLessonsFragmentTest.kt
@BenHenning BenHenning added the Z-ibt Temporary label for Ben to keep track of issues he's triaged. label Sep 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue This item is good for new contributors to make their pull request. Z-ibt Temporary label for Ben to keep track of issues he's triaged.
4 participants