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

Catch errors at the time of javascript compilation #14849

Merged
merged 5 commits into from
Feb 15, 2022

Conversation

Hudda
Copy link
Contributor

@Hudda Hudda commented Feb 3, 2022

Overview

1. This PR fixes or fixes part of #[fill_in_number_here].
2. This PR does the following: Use check to catch errors during javascript compilation.

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".

Proof that changes are correct

Screenshot 2022-02-03 at 8 51 15 PM

PR Pointers

  • Make sure to follow the instructions for making a code change.
  • 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 can assign anyone for review/help if you leave a comment like the following: "{{Question/comment}} @{{reviewer_username}} PTAL"
  • Some of the e2e tests are flaky, and can fail for reasons unrelated to your PR. We are working on fixing this, but in the meantime, if you need to restart the tests, please check the "If your build fails" wiki page.

@oppiabot
Copy link

oppiabot bot commented Feb 3, 2022

Hi @Hudda, can you complete the following:

  1. The proof that changes are correct has not been provided, please make sure to upload a image/video showing that the changes are correct. Or include a sentence saying "No proof of changes needed because" and the reason why proof of changes cannot be provided.
    Thanks!

@oppiabot oppiabot bot assigned Hudda Feb 3, 2022
@oppiabot
Copy link

oppiabot bot commented Feb 3, 2022

Hi, @Hudda, this pull request does not have a "CHANGELOG: ..." label as mentioned in the PR pointers. Assigning @Hudda to add the required label. PRs without this label will not be merged. If you are unsure of which label to add, please ask the reviewers for guidance. Thanks!

@Hudda Hudda assigned DubeySandeep and unassigned Hudda Feb 3, 2022
@oppiabot oppiabot bot added the PR: don't merge - STALE BUILD The build on this PR is stale and should be restarted. label Feb 5, 2022
@oppiabot
Copy link

oppiabot bot commented Feb 5, 2022

Hi @Hudda, the build of this PR is stale and this could result in tests failing in develop. Please update this pull request with the latest changes from develop. Thanks!

Copy link
Member

@DubeySandeep DubeySandeep left a comment

Choose a reason for hiding this comment

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

@Hudda Thanks for the PR, do we need a backend test for this changes? (The coverage check is failing o this PR)

@DubeySandeep DubeySandeep assigned Hudda and unassigned DubeySandeep Feb 8, 2022
@oppiabot oppiabot bot removed the PR: don't merge - STALE BUILD The build on this PR is stale and should be restarted. label Feb 12, 2022
@Hudda
Copy link
Contributor Author

Hudda commented Feb 12, 2022

Added tests, @DubeySandeep PTAL!

@Hudda Hudda assigned DubeySandeep and unassigned Hudda Feb 12, 2022
@oppiabot
Copy link

oppiabot bot commented Feb 12, 2022

Hi @Hudda, there is a new change in develop which needs to be in your PR. Please update your branch with the latest changes in develop. For instructions, refer to this link. Thanks!

@oppiabot oppiabot bot assigned Hudda Feb 12, 2022
@Hudda Hudda removed their assignment Feb 14, 2022
@oppiabot
Copy link

oppiabot bot commented Feb 15, 2022

Unassigning @DubeySandeep since they have already approved the PR.

@oppiabot oppiabot bot added the PR: LGTM label Feb 15, 2022
@oppiabot
Copy link

oppiabot bot commented Feb 15, 2022

Hi @Hudda, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to merge this PR once the CI checks pass and you're happy with it. Thanks!

@oppiabot oppiabot bot assigned Hudda Feb 15, 2022
@Hudda Hudda merged commit 9df1ad5 into oppia:develop Feb 15, 2022
@Hudda Hudda deleted the catch-compile-errors branch February 15, 2022 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants