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

Fix #8869: Add innerHTML lint check #8870

Merged
merged 10 commits into from
Mar 28, 2020
Merged

Fix #8869: Add innerHTML lint check #8870

merged 10 commits into from
Mar 28, 2020

Conversation

kevintab95
Copy link
Member

Overview

  1. This PR fixes or fixes part of Add a lint check to forbid the use of innerHTML #8869 .
  2. This PR does the following: This PR adds a lint check for innerHTML

Essential Checklist

  • 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: ...".)
  • The linter/Karma presubmit checks have passed locally on your machine.
  • "Allow edits from maintainers" is checked. (See here for instructions on how to enable it.)
    • This lets reviewers restart your CircleCI tests for you.
  • The PR is made from a branch that's not called "develop".

PR Pointers

  • Oppiabot will notify you when you don't add a PR_CHANGELOG label. If you are unable to do so, please @-mention a code owner (who will be in the Reviewers list), or ask on Gitter.
  • For what code owners will expect, see the Code Owner's wiki page.
  • Make sure your PR follows conventions in the style guide, otherwise this will lead to review delays
  • Never force push. If you do, your PR will be closed.

@oppiabot
Copy link

oppiabot bot commented Mar 18, 2020

Assigning @kevinlee12 for the first-pass review of this pull request. Thanks!

@kevintab95 kevintab95 requested a review from seanlip March 18, 2020 22:22
seanlip
seanlip approved these changes Mar 18, 2020
View reviewed changes
Copy link
Member

@seanlip seanlip left a comment

Choose a reason for hiding this comment

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

Thanks @kevintab95!

@Hudda please make sure to incorporate these changes into your PR -- thanks!

@seanlip
Copy link
Member

seanlip commented Mar 18, 2020

(Although, @kevintab95 -- CI checks are failing, PTAL.)

@seanlip seanlip mentioned this pull request Mar 18, 2020
Merged
8 tasks
kevinlee12
kevinlee12 approved these changes Mar 19, 2020
View reviewed changes
Copy link
Contributor

@kevinlee12 kevinlee12 left a comment

Choose a reason for hiding this comment

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

lgtm!

@kevinlee12 kevinlee12 assigned kevintab95 and unassigned kevinlee12 and seanlip Mar 19, 2020
vojtechjelinek
vojtechjelinek approved these changes Mar 19, 2020
View reviewed changes
Copy link
Contributor

@vojtechjelinek vojtechjelinek left a comment

Choose a reason for hiding this comment

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

LGTM from code owner's perspective. Thanks!

ankita240796
ankita240796 approved these changes Mar 19, 2020
View reviewed changes
Copy link
Contributor

@ankita240796 ankita240796 left a comment

Choose a reason for hiding this comment

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

LGTM from code-owner's perspective, Thanks @kevintab95!

@ankita240796 ankita240796 removed their assignment Mar 19, 2020
aks681
aks681 approved these changes Mar 19, 2020
View reviewed changes
Copy link
Contributor

@aks681 aks681 left a comment

Choose a reason for hiding this comment

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

Lgtm as codeowner.

@aks681 aks681 removed their assignment Mar 19, 2020
@kevintab95
Copy link
Member Author

Pushed a commit to resolve the lint_test failures from earlier.

@oppiabot
Copy link

oppiabot bot commented Mar 21, 2020

Hi @kevintab95. Due to recent changes in the "develop" branch, this PR now has a merge conflict. Please follow this link if you need help resolving the conflict, so that the PR can be merged. Thanks!

@kevinlee12
Copy link
Contributor

@kevintab95 ptal!

@kevintab95 kevintab95 mentioned this pull request Mar 24, 2020
Merged
4 tasks
@kevintab95 kevintab95 requested a review from Hudda as a code owner March 26, 2020 15:13
@kevintab95 kevintab95 assigned Hudda and unassigned kevintab95 Mar 26, 2020
@kevintab95
Copy link
Member Author

Hi @Hudda, PTAL as code-owner!

Hudda
Hudda approved these changes Mar 26, 2020
View reviewed changes
Copy link
Contributor

@Hudda Hudda left a comment

Choose a reason for hiding this comment

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

LGTM from code-owner's perspective, Thanks @kevintab95!

@Hudda Hudda assigned kevintab95 and unassigned Hudda Mar 26, 2020
@oppiabot
Copy link

oppiabot bot commented Mar 27, 2020

Hi @kevintab95. Due to recent changes in the "develop" branch, this PR now has a merge conflict. Please follow this link if you need help resolving the conflict, so that the PR can be merged. Thanks!

@kevinlee12 kevinlee12 merged commit 7ec1258 into oppia:develop Mar 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Hudda Hudda Hudda approved these changes

@aks681 aks681 aks681 approved these changes

@ankita240796 ankita240796 ankita240796 approved these changes

@seanlip seanlip seanlip approved these changes

@vojtechjelinek vojtechjelinek vojtechjelinek approved these changes

@kevinlee12 kevinlee12 kevinlee12 approved these changes

Assignees

@kevintab95 kevintab95

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@kevintab95 @seanlip @kevinlee12 @vojtechjelinek @ankita240796 @aks681 @Hudda