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 regex to ban <include> in layout files #3746

Closed
rt4914 opened this issue Aug 30, 2021 · 0 comments · Fixed by #4656
Closed

Add regex to ban <include> in layout files #3746

rt4914 opened this issue Aug 30, 2021 · 0 comments · Fixed by #4656
Assignees
Labels
Impact: Low Low perceived user impact (e.g. edge cases). Priority: Nice-to-have This work item is nice to have for its milestone. Type: Task A single task of work corresponding to a greater milestone. Generally corresponds to a single PR. Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@rt4914
Copy link
Contributor

rt4914 commented Aug 30, 2021

Add regex to ban in layout files

This can be done in this file : https://github.com/oppia/oppia-android/blob/develop/scripts/assets/file_content_validation_checks.textproto

@rt4914 rt4914 added Priority: Nice-to-have This work item is nice to have for its milestone. Type: Task A single task of work corresponding to a greater milestone. Generally corresponds to a single PR. labels Aug 30, 2021
@rt4914 rt4914 added this to the Backlog milestone Aug 30, 2021
@Broppia Broppia added issue_type_infrastructure Impact: Low Low perceived user impact (e.g. edge cases). labels Jul 7, 2022
@BenHenning BenHenning added Z-ibt Temporary label for Ben to keep track of issues he's triaged. issue_user_developer labels Sep 15, 2022
@BenHenning BenHenning removed this from the Backlog milestone Sep 16, 2022
BenHenning added a commit that referenced this issue Nov 8, 2022
<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
Fixes  [#3746](#3746)

This PR bans all usage of the <include...> tag in XML layout files that
reference it. It does so by adding a check block inside the
file_content_validation_checks text proto script file.

This pr also removes the toolbar.xml file, since its no longer being
used.
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] 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: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-(A11y)-Guide))
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

## SCREENSHOT FOR THE REGEX PATTERN CHECKS FAILED 


![checksfailed](https://user-images.githubusercontent.com/31986629/196036816-ddb6e065-d4bc-4f96-95e7-f5228ae3e23d.PNG)

## SCREENSHOT FOR THE REGEX PATTERN CHECKS PASSED


![passed](https://user-images.githubusercontent.com/31986629/196038476-e54e2319-6567-4b96-934d-c040b36360c5.PNG)

## A LIST OF THE FILES THAT HAVE THE <INCLUDE.../> TAG
- app/src/main/res/layout/add_profile_activity.xml
- addprofileactivity
- app/src/main/res/layout/administrator_controls_activity.xml:15: Remove
<include .../> tag from layout and use the appropriate widget, e.g like
AppBarLayout widget instead
- app/src/main/res/layout/help_without_drawer_activity.xml:11: Remove
<include .../> tag from layout and use the appropriate widget, e.g like
AppBarLayout widget instead
- app/src/main/res/layout/option_activity.xml:13: Remove <include .../>
tag from layout and use the appropriate widget, e.g like AppBarLayout
widget instead
- app/src/main/res/layout/options_without_drawer_activity.xml:11: Remove
<include .../> tag from layout and use the appropriate widget, e.g like
AppBarLayout widget instead
- app/src/main/res/layout/developer_options_activity.xml:15: Remove
<include .../> tag from layout and use the appropriate widget, e.g like
AppBarLayout widget instead
- app/src/main/res/layout/home_activity.xml:14: Remove <include .../>
tag from layout and use the appropriate widget, e.g like AppBarLayout
widget instead
- app/src/main/res/layout/help_activity.xml:13: Remove <include .../>
tag from layout and use the appropriate widget, e.g like AppBarLayout
widget instead
- app/src/main/res/layout/profile_progress_activity.xml:9: Remove
<include .../> tag from layout and use the appropriate widget, e.g like
AppBarLayout widget instead
-
app/src/main/res/layout-sw600dp/administrator_controls_activity.xml:15:
Remove <include .../> tag from layout and use the appropriate widget,
e.g like AppBarLayout widget instead
- app/src/main/res/layout-sw600dp/option_activity.xml:13: Remove
<include .../> tag from layout and use the appropriate widget, e.g like
AppBarLayout widget instead
-
app/src/main/res/layout-sw600dp/options_without_drawer_activity.xml:12:
Remove <include .../> tag from layout and use the appropriate widget,
e.g like AppBarLayout widget instead
- app/src/main/res/layout-sw600dp/help_activity.xml:13: Remove <include
.../> tag from layout and use the appropriate widget, e.g like
AppBarLayout widget instead

Co-authored-by: MAZAKPE <[email protected]>
Co-authored-by: Ben Henning <[email protected]>
Repository owner moved this from Needs Triage to Done in [Team] Developer Workflow & Infrastructure - Android Nov 8, 2022
adhiamboperes pushed a commit to adhiamboperes/oppia-android that referenced this issue Nov 10, 2022
…a#4656)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
Fixes  [oppia#3746](oppia#3746)

This PR bans all usage of the <include...> tag in XML layout files that
reference it. It does so by adding a check block inside the
file_content_validation_checks text proto script file.

This pr also removes the toolbar.xml file, since its no longer being
used.
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] 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: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-(A11y)-Guide))
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

## SCREENSHOT FOR THE REGEX PATTERN CHECKS FAILED 


![checksfailed](https://user-images.githubusercontent.com/31986629/196036816-ddb6e065-d4bc-4f96-95e7-f5228ae3e23d.PNG)

## SCREENSHOT FOR THE REGEX PATTERN CHECKS PASSED


![passed](https://user-images.githubusercontent.com/31986629/196038476-e54e2319-6567-4b96-934d-c040b36360c5.PNG)

## A LIST OF THE FILES THAT HAVE THE <INCLUDE.../> TAG
- app/src/main/res/layout/add_profile_activity.xml
- addprofileactivity
- app/src/main/res/layout/administrator_controls_activity.xml:15: Remove
<include .../> tag from layout and use the appropriate widget, e.g like
AppBarLayout widget instead
- app/src/main/res/layout/help_without_drawer_activity.xml:11: Remove
<include .../> tag from layout and use the appropriate widget, e.g like
AppBarLayout widget instead
- app/src/main/res/layout/option_activity.xml:13: Remove <include .../>
tag from layout and use the appropriate widget, e.g like AppBarLayout
widget instead
- app/src/main/res/layout/options_without_drawer_activity.xml:11: Remove
<include .../> tag from layout and use the appropriate widget, e.g like
AppBarLayout widget instead
- app/src/main/res/layout/developer_options_activity.xml:15: Remove
<include .../> tag from layout and use the appropriate widget, e.g like
AppBarLayout widget instead
- app/src/main/res/layout/home_activity.xml:14: Remove <include .../>
tag from layout and use the appropriate widget, e.g like AppBarLayout
widget instead
- app/src/main/res/layout/help_activity.xml:13: Remove <include .../>
tag from layout and use the appropriate widget, e.g like AppBarLayout
widget instead
- app/src/main/res/layout/profile_progress_activity.xml:9: Remove
<include .../> tag from layout and use the appropriate widget, e.g like
AppBarLayout widget instead
-
app/src/main/res/layout-sw600dp/administrator_controls_activity.xml:15:
Remove <include .../> tag from layout and use the appropriate widget,
e.g like AppBarLayout widget instead
- app/src/main/res/layout-sw600dp/option_activity.xml:13: Remove
<include .../> tag from layout and use the appropriate widget, e.g like
AppBarLayout widget instead
-
app/src/main/res/layout-sw600dp/options_without_drawer_activity.xml:12:
Remove <include .../> tag from layout and use the appropriate widget,
e.g like AppBarLayout widget instead
- app/src/main/res/layout-sw600dp/help_activity.xml:13: Remove <include
.../> tag from layout and use the appropriate widget, e.g like
AppBarLayout widget instead

Co-authored-by: MAZAKPE <[email protected]>
Co-authored-by: Ben Henning <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Impact: Low Low perceived user impact (e.g. edge cases). Priority: Nice-to-have This work item is nice to have for its milestone. Type: Task A single task of work corresponding to a greater milestone. Generally corresponds to a single PR. Z-ibt Temporary label for Ben to keep track of issues he's triaged.
Development

Successfully merging a pull request may close this issue.

4 participants