Fix #603 refactor play to lessons #612

PrarabdhGarg · 2020-01-21T05:11:15Z

Fixes #603

Explanation

This pull request completes the refactoring process, refactoring files that were left in the previous pull request.

Checklist

[x ] The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
[x ] The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
[x ] The PR follows the style guide.
[ x] The PR does not contain any unnecessary auto-generated code from Android Studio.
[ x] The PR is made from a branch that's not called "develop".
[x ] The PR is made from a branch that is up-to-date with "develop".
[ x] The PR's branch is based on "develop" and not on any other branch.
[ x] The PR is assigned to an appropriate reviewer in both the Assignees and the Reviewers sections.

As per the new mocks, the paly tab was renamed to lessons. Hence, the entrire project was refactored, along with tests, to maintain consistency with mocks. Resolves oppia#603

Partly resolves oppia#603

rt4914 · 2020-01-21T05:19:28Z

@PrarabdhGarg Why was this PR created, as #609 is similar to this one, any specific reason? Also, please close one of these PR's with proper comment of why you are closing it.

PrarabdhGarg · 2020-01-21T05:26:47Z

@rt4914 is it possible to update the previous pull request with the latest changes I have made?

rt4914 · 2020-01-21T05:33:00Z

@rt4914 is it possible to update the previous pull request with the latest changes I have made?

Yes it is possible, you just need to make a new commit to that same branch and update it.
For example, your branch name is my-sample-branch. So you do following things,

git checkout my-sample-branch
Now you are in your branch, so make all the changes that you need. After all the changes are finished.

git add .
git commit -m "appropriate commit message"
git push origin my-sample-branch

PrarabdhGarg · 2020-01-21T05:38:13Z

@rt4914 I was not aware of this. Will follow this going forward.
In this case, I had to create a new pull request as you had suggested changing the branch name

PrarabdhGarg · 2020-01-21T05:38:30Z

I will close the previous Pull request

rt4914 · 2020-01-21T05:40:14Z

@rt4914 I was not aware of this. Will follow this going forward.
In this case, I had to create a new pull request as you had suggested changing the branch name

Have you taken a look at this document: https://github.com/oppia/oppia-android/wiki#instructions-for-making-a-code-change

This might help.

PrarabdhGarg · 2020-01-21T05:41:21Z

@rt4914 I was not aware of this. Will follow this going forward.
In this case, I had to create a new pull request as you had suggested changing the branch name

Have you taken a look at this document: https://github.com/oppia/oppia-android/wiki#instructions-for-making-a-code-change

This might help.

I will surely read this. Thanks a lot

PrarabdhGarg · 2020-01-21T06:23:38Z

@rt4914 I have closed the previous pull request

rt4914 · 2020-01-21T06:25:38Z

@rt4914 I have closed the previous pull request

@PrarabdhGarg Thanks.

rt4914

Looks good to me, some final changes are needed.

-> Address all the comments and fix those issues.
-> change play_chapter_view to lessons_chapter_view
-> Second step change will need to update the code in ChapterSummaryAdapter.kt

rt4914 · 2020-01-21T08:53:09Z

.idea/codeStyles/Project.xml

@@ -157,4 +162,4 @@
      </indentOptions>
    </codeStyleSettings>
  </code_scheme>
-</component>
+</component>


Revert this file. We do not want these changes.

@PrarabdhGarg This file is still here. We do not want to make any changes to this file. So please revert back this file to its original code.

app/src/main/java/org/oppia/app/topic/lessons/TopicLessonsFragment.kt

PrarabdhGarg · 2020-01-22T04:58:32Z

@rt4914 I have made the required changes

…ub.com/PrarabdhGarg/oppia-android into fix-oppia#603-refactor-play-to-lessons

rt4914

@PrarabdhGarg .idea/codeStyles/Project.xml was not meant to be be deleted. In my earlier comment, by revert I meant that the changes should be reverted and finally the file should be same as before on develop branch.

Also revert .idea/vcs.xml file changes too, we do not need those changes.

rt4914 · 2020-01-22T05:40:04Z

.idea/vcs.xml

  </component>
-</project>
+</project>


Please revert this file changes. We do not want these unnecessary changes.

I will do this. I am sorry for the confusion

No issues. To make PRs more effective, before sending the PR for review its nice to go to the pull request, click on "Files Changed" and try to self review it first and whatever mistakes you find, fix them and then send it for review. This will help you a lot in becoming a good developer.

Same here. This file changes are also not needed.

PrarabdhGarg · 2020-01-27T04:56:09Z

@rt4914 I have added the files again and made the small changes

PrarabdhGarg · 2020-01-27T04:57:57Z

Are the merge conflicts coming because I didn't update my branch with the latest develop branch? If yes, should I be solving them, or you would be solving them while merging the pull request?

rt4914 · 2020-01-27T05:18:41Z

Are the merge conflicts coming because I didn't update my branch with the latest develop branch? If yes, should I be solving them, or you would be solving them while merging the pull request?

It would be nice if we can solve it right now only.

rt4914

@PrarabdhGarg PTAL

rt4914 · 2020-01-27T05:19:46Z

.idea/codeStyles/Project.xml

@@ -157,4 +162,4 @@
      </indentOptions>
    </codeStyleSettings>
  </code_scheme>
-</component>
+</component>


@PrarabdhGarg This file is still here. We do not want to make any changes to this file. So please revert back this file to its original code.

rt4914 · 2020-01-27T05:20:05Z

.idea/vcs.xml

  </component>
-</project>
+</project>


Same here. This file changes are also not needed.

PrarabdhGarg · 2020-01-27T05:35:42Z

@PrarabdhGarg PTAL

@rt4914 Since I had deleted the files in my previous commit, I just copied the files from the current develop branch, and committed them. I guess now they are the same as that on the develop branch

Sync forked repository

PrarabdhGarg · 2020-01-27T05:50:06Z

@rt4914 I have also resolved the merge conflicts

rt4914 · 2020-01-27T06:16:26Z

@PrarabdhGarg PTAL

@rt4914 Since I had deleted the files in my previous commit, I just copied the files from the current develop branch, and committed them. I guess now they are the same as that on the develop branch

You can go to Files changed tab in this PR and check for following files:
.idea/codeStyles/Project.xml
.idea/vcs.xml

These files are getting changed. We do not want any change to these files.

rt4914 · 2020-01-27T06:16:42Z

@rt4914 I have also resolved the merge conflicts

@PrarabdhGarg thanks.

PrarabdhGarg · 2020-01-27T07:27:17Z

@rt4914 I have reverted the files. How do I ensure that any local changes in these files are not pushed in the future? Should I add them to the git ignore file?

rt4914 · 2020-01-27T08:01:55Z

@rt4914 I have reverted the files. How do I ensure that any local changes in these files are not pushed in the future? Should I add them to the git ignore file?

Thanks a lot. Actually these files should not be added to git ignore. Also, mostly it should get fixed in your next PR. If it does not get fixed please inform us and we will try to come up with some solution.

rt4914

LGTM. Thanks @PrarabdhGarg

PrarabdhGarg added 3 commits January 18, 2020 16:27

Refactor play to lessons

7fa2223

As per the new mocks, the paly tab was renamed to lessons. Hence, the entrire project was refactored, along with tests, to maintain consistency with mocks. Resolves oppia#603

Refactor remaining instances of play with lessons

1802d08

Partly resolves oppia#603

Solve errors in the privious commit

103d7ef

PrarabdhGarg requested a review from rt4914 January 21, 2020 05:11

PrarabdhGarg mentioned this pull request Jan 21, 2020

Refactor play to lessons #609

Closed

2 tasks

rt4914 suggested changes Jan 21, 2020

View reviewed changes

PrarabdhGarg added 2 commits January 22, 2020 10:15

Additional Refactoring

73e23cb

Delete Project.xml

120ecd9

PrarabdhGarg added 2 commits January 22, 2020 10:29

Minor refactoring

68e98fc

Merge branch 'fix-oppia#603-refactor-play-to-lessons' of https://gith…

47c25c9

…ub.com/PrarabdhGarg/oppia-android into fix-oppia#603-refactor-play-to-lessons

rt4914 suggested changes Jan 22, 2020

View reviewed changes

rt4914 assigned PrarabdhGarg Jan 23, 2020

Add files deleted prviously by mistake

769e692

rt4914 suggested changes Jan 27, 2020

View reviewed changes

PrarabdhGarg added 2 commits January 27, 2020 11:14

Merge pull request #1 from oppia/develop

710093a

Sync forked repository

Merge with current develop branch

a5438db

PrarabdhGarg added 4 commits January 27, 2020 12:51

Update vcs.xml

bdd17b0

Update Project.xml

d657e1f

Update Project.xml

c1675d6

Update Project.xml

1632ae3

rt4914 approved these changes Jan 27, 2020

View reviewed changes

rt4914 merged commit ae5a0c4 into oppia:develop Jan 27, 2020

PrarabdhGarg deleted the fix-#603-refactor-play-to-lessons branch February 22, 2020 12:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #603 refactor play to lessons #612

Fix #603 refactor play to lessons #612

PrarabdhGarg commented Jan 21, 2020

rt4914 commented Jan 21, 2020

PrarabdhGarg commented Jan 21, 2020

rt4914 commented Jan 21, 2020

PrarabdhGarg commented Jan 21, 2020

PrarabdhGarg commented Jan 21, 2020

rt4914 commented Jan 21, 2020

PrarabdhGarg commented Jan 21, 2020

PrarabdhGarg commented Jan 21, 2020

rt4914 commented Jan 21, 2020

rt4914 left a comment

rt4914 Jan 21, 2020

rt4914 Jan 27, 2020

PrarabdhGarg commented Jan 22, 2020

rt4914 left a comment

rt4914 Jan 22, 2020

PrarabdhGarg Jan 23, 2020

rt4914 Jan 23, 2020

rt4914 Jan 27, 2020

PrarabdhGarg commented Jan 27, 2020

PrarabdhGarg commented Jan 27, 2020

rt4914 commented Jan 27, 2020

rt4914 left a comment

rt4914 Jan 27, 2020

rt4914 Jan 27, 2020

PrarabdhGarg commented Jan 27, 2020

PrarabdhGarg commented Jan 27, 2020

rt4914 commented Jan 27, 2020

rt4914 commented Jan 27, 2020

PrarabdhGarg commented Jan 27, 2020

rt4914 commented Jan 27, 2020

rt4914 left a comment

Fix #603 refactor play to lessons #612

Fix #603 refactor play to lessons #612

Conversation

PrarabdhGarg commented Jan 21, 2020

Explanation

Checklist

rt4914 commented Jan 21, 2020

PrarabdhGarg commented Jan 21, 2020

rt4914 commented Jan 21, 2020

PrarabdhGarg commented Jan 21, 2020

PrarabdhGarg commented Jan 21, 2020

rt4914 commented Jan 21, 2020

PrarabdhGarg commented Jan 21, 2020

PrarabdhGarg commented Jan 21, 2020

rt4914 commented Jan 21, 2020

rt4914 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PrarabdhGarg commented Jan 22, 2020

rt4914 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PrarabdhGarg commented Jan 27, 2020

PrarabdhGarg commented Jan 27, 2020

rt4914 commented Jan 27, 2020

rt4914 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PrarabdhGarg commented Jan 27, 2020

PrarabdhGarg commented Jan 27, 2020

rt4914 commented Jan 27, 2020

rt4914 commented Jan 27, 2020

PrarabdhGarg commented Jan 27, 2020

rt4914 commented Jan 27, 2020

rt4914 left a comment

Choose a reason for hiding this comment