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.

Sign up for GitHub

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

Jump to bottom

Merge story_fragment.xml into single xml file #3576 #3995

Merged
merged 9 commits into from
Jan 7, 2022
Merged

Merge story_fragment.xml into single xml file #3576 #3995

merged 9 commits into from
Jan 7, 2022

Conversation

shivambh12
Copy link
Contributor

@shivambh12 shivambh12 commented Nov 17, 2021 •
edited

Explanation
Fixes #3576

Essential Checklist
The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
Any changes to scripts/assets files have their rationale included in the PR explanation.
The PR follows the style guide.
The PR does not contain any unnecessary code changes from Android Studio (reference).
The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
The PR is assigned to the appropriate reviewers (reference).

Before Changes
Phone
After Changes
Phone(1)
Before Changes
Phone-land
After Changes
Phone-land(1)
Before Changes
Tablet
After Changes
Tablet(1)
Before Changes
Tablet-land
After Changes
Tablet-land(1)

@shivambh12
Merge story_fragment.xml into single xml file oppia#3576
9ed6f6e 
Removed story_fragment-land and story_fragment-sw600dp
Applied Required Changes in dimens.xml
@shivambh12
Copy link
Contributor Author

@Arjupta PTAL

rt4914
rt4914 suggested changes Nov 18, 2021
View reviewed changes
Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

PTAL.

app/src/main/res/values/dimens.xml Outdated Show resolved Hide resolved
@rt4914 rt4914 assigned shivambh12 and unassigned Arjupta Nov 18, 2021
@shivambh12
Added layout-sw600dp/story_fragment.xml
19bc7fb 
And Applied Required Changes to dimen.xml
@shivambh12
Copy link
Contributor Author

@rt4914 PTAL

@oppiabot oppiabot bot assigned rt4914 and unassigned shivambh12 Nov 19, 2021
@oppiabot
Copy link

oppiabot bot commented Nov 19, 2021

Unassigning @shivambh12 since a re-review was requested. @shivambh12, please make sure you have addressed all review comments. Thanks!

@rt4914 rt4914 assigned Arjupta and unassigned rt4914 Nov 19, 2021
@rt4914
Copy link
Contributor

rt4914 commented Nov 19, 2021

@Arjupta Assigning you first. Thanks.

@Arjupta
Copy link
Contributor

Arjupta commented Nov 23, 2021

I will be able to review this PR by weekend.

@rt4914 rt4914 self-assigned this Nov 24, 2021
rt4914
rt4914 suggested changes Nov 26, 2021
View reviewed changes
Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

@shivambh12 PTAL

app/src/main/res/values-land/dimens.xml Outdated Show resolved Hide resolved
app/src/main/res/values-land/dimens.xml Outdated Show resolved Hide resolved
app/src/main/res/values-land/dimens.xml Outdated Show resolved Hide resolved
app/src/main/res/values-sw600dp-land/dimens.xml Outdated Show resolved Hide resolved
app/src/main/res/values-sw600dp-land/dimens.xml Outdated Show resolved Hide resolved
app/src/main/res/values-sw600dp-port/dimens.xml Outdated Show resolved Hide resolved
app/src/main/res/values-sw600dp-port/dimens.xml Outdated Show resolved Hide resolved
app/src/main/res/values/dimens.xml Outdated Show resolved Hide resolved
app/src/main/res/values/dimens.xml Outdated Show resolved Hide resolved
@rt4914 rt4914 assigned shivambh12 and unassigned rt4914 Nov 26, 2021
@shivambh12
Copy link
Contributor Author

@rt4914 PTAL

@oppiabot oppiabot bot assigned rt4914 and unassigned shivambh12 Nov 27, 2021
@oppiabot
Copy link

oppiabot bot commented Nov 27, 2021

Unassigning @shivambh12 since a re-review was requested. @shivambh12, please make sure you have addressed all review comments. Thanks!

@Arjupta Arjupta removed their assignment Nov 30, 2021
rt4914
rt4914 suggested changes Dec 1, 2021
View reviewed changes
Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

@shivambh12 Left some comments.

Also please make sure you reply to all the comments.

app/src/main/res/values-sw600dp-land/dimens.xml Outdated Show resolved Hide resolved
app/src/main/res/values-sw600dp-port/dimens.xml Outdated Show resolved Hide resolved
app/src/main/res/values/dimens.xml Outdated Show resolved Hide resolved
app/src/main/res/values-land/dimens.xml Outdated Show resolved Hide resolved
app/src/main/res/values-land/dimens.xml Outdated Show resolved Hide resolved
@rt4914 rt4914 removed their assignment Dec 1, 2021
@shivambh12
Copy link
Contributor Author

@Arjupta PTAL

@oppiabot oppiabot bot assigned Arjupta and unassigned shivambh12 Dec 10, 2021
@oppiabot
Copy link

oppiabot bot commented Dec 10, 2021

Unassigning @shivambh12 since a re-review was requested. @shivambh12, please make sure you have addressed all review comments. Thanks!

@BenHenning
Copy link
Sponsor Member

Hi. As of today, some main reviewers have taken time off for the next few weeks, so it may take a little while before we can look at this PR. We appreciate your patience while some of our team members recharge. We'll be fully returning on 4 January 2021.

Arjupta
Arjupta approved these changes Dec 20, 2021
View reviewed changes
Copy link
Contributor

@Arjupta Arjupta left a comment

Choose a reason for hiding this comment

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

Thanks @shivambh12 PR looks good to me.

@oppiabot oppiabot bot unassigned Arjupta Dec 20, 2021
@oppiabot
Copy link

oppiabot bot commented Dec 20, 2021

Unassigning @Arjupta since they have already approved the PR.

@oppiabot
Copy link

oppiabot bot commented Dec 20, 2021

Assigning @rt4914 for code owner reviews. Thanks!

@oppiabot
Copy link

oppiabot bot commented Dec 27, 2021

Hi @shivambh12, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

@oppiabot oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Dec 27, 2021
@shivambh12
Copy link
Contributor Author

@rt4914 PTAL

@oppiabot oppiabot bot removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Dec 28, 2021
rt4914
rt4914 suggested changes Jan 3, 2022
View reviewed changes
Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

@shivambh12 Thanks. Suggested nit change

app/src/main/res/values/dimens.xml Outdated Show resolved Hide resolved
@rt4914 rt4914 assigned shivambh12 and unassigned rt4914 Jan 3, 2022
@shivambh12
Copy link
Contributor Author

@rt4914 PTAL

@oppiabot oppiabot bot assigned rt4914 and unassigned shivambh12 Jan 7, 2022
@oppiabot
Copy link

oppiabot bot commented Jan 7, 2022

Unassigning @shivambh12 since a re-review was requested. @shivambh12, please make sure you have addressed all review comments. Thanks!

rt4914
rt4914 approved these changes Jan 7, 2022
View reviewed changes
Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@rt4914 rt4914 enabled auto-merge (squash) January 7, 2022 19:10
@oppiabot oppiabot bot unassigned rt4914 Jan 7, 2022
@oppiabot
Copy link

oppiabot bot commented Jan 7, 2022

Unassigning @rt4914 since they have already approved the PR.

@oppiabot oppiabot bot added the PR: LGTM label Jan 7, 2022
@rt4914 rt4914 merged commit 8a86210 into oppia:develop Jan 7, 2022
@shivambh12 shivambh12 deleted the New branch January 10, 2022 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rt4914 rt4914 rt4914 approved these changes

@Arjupta Arjupta Arjupta approved these changes

Assignees
No one assigned
Labels
PR: LGTM
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Merge story_fragment.xml into single xml file
4 participants
@shivambh12 @rt4914 @Arjupta @BenHenning