Fix part of #10474: Cover more files with strict checks #11100

krishita30j · 2020-10-31T16:44:20Z

…e.spec.ts, id-generation.service.spec.ts and tsconfig-strict.json

Overview

This PR fixes or fixes part of Make typescript checks strict #10474.
This PR does the following: Cover more files with strict checks

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

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.

…e.spec.ts, id-generation.service.spec.ts and tsconfig-strict.json

vojtechjelinek

Thanks! Just a few comments.

vojtechjelinek · 2020-10-31T17:04:52Z

core/templates/services/code-normalizer.service.ts

+        // eslint-disable-next-line max-len
+        existingNumSpaces = Object.keys(numSpacesToDesiredIndentLevel).map(Number);


We prefer to limit the usage of eslint-disable exceptions. You can do something like this:

Suggested change

// eslint-disable-next-line max-len

existingNumSpaces = Object.keys(numSpacesToDesiredIndentLevel).map(Number);

existingNumSpaces = Object.keys(

numSpacesToDesiredIndentLevel

).map(Number);

vojtechjelinek · 2020-10-31T17:05:21Z

core/templates/services/code-normalizer.service.ts

+      // eslint-disable-next-line max-len
+      var existingNumSpaces = Object.keys(numSpacesToDesiredIndentLevel).map(Number);


We prefer to limit the usage of eslint-disable exceptions. You can do something like this:

Suggested change

// eslint-disable-next-line max-len

var existingNumSpaces = Object.keys(numSpacesToDesiredIndentLevel).map(Number);

var existingNumSpaces = Object.keys(

numSpacesToDesiredIndentLevel

).map(Number);

vojtechjelinek · 2020-10-31T17:06:09Z

core/templates/services/code-normalizer.service.ts

@@ -52,12 +52,12 @@ export class CodeNormalizerService {
    var FOUR_SPACES = '    ';
    // Maps the number of spaces at the beginning of a line to an int
    // specifying the desired indentation level.
-    var numSpacesToDesiredIndentLevel = {
+    var numSpacesToDesiredIndentLevel: { [key: number]: number; } = {


Are you sure about the ; inside the type definition?

Suggested change

var numSpacesToDesiredIndentLevel: { [key: number]: number; } = {

var numSpacesToDesiredIndentLevel: { [key: number]: number } = {

oppiabot · 2020-10-31T17:09:27Z

Hi @krishita30j, it looks like some changes were requested on this pull request by @vojtechjelinek. PTAL. Thanks!

krishita30j · 2020-10-31T17:26:45Z

@vojtechjelinek, I have made the changes.

vojtechjelinek · 2020-10-31T19:31:43Z

@krishita30j Please respond to all reviewer's comments with: "Done" when you fix them (next time).

vojtechjelinek

LGTM!

nishantwrp

Lgtm

oppiabot · 2020-10-31T19:37:11Z

Unassigning @nishantwrp since they have already approved the PR.

aks681

Lgtm as codeowner.

oppiabot · 2020-11-02T04:09:52Z

Hi @krishita30j. 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!

oppiabot · 2020-11-02T22:23:13Z

Hi @krishita30j. 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!

oppiabot · 2020-11-05T17:21:27Z

Unassigning @nithusha21 since they have already approved the PR.

oppiabot · 2020-11-09T06:02:30Z

Hi @krishita30j. 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!

Modified code-normalizer.service.ts, construct-translation-ids.servic…

1f7e57e

…e.spec.ts, id-generation.service.spec.ts and tsconfig-strict.json

krishita30j requested review from aks681, nishantwrp, srijanreddy98 and vojtechjelinek as code owners October 31, 2020 16:44

vojtechjelinek requested changes Oct 31, 2020

View reviewed changes

oppiabot bot assigned krishita30j Oct 31, 2020

Modified code-normalizer.service.ts

690c1f7

vojtechjelinek approved these changes Oct 31, 2020

View reviewed changes

vojtechjelinek assigned aks681, srijanreddy98 and nishantwrp and unassigned krishita30j Oct 31, 2020

nishantwrp approved these changes Oct 31, 2020

View reviewed changes

oppiabot bot unassigned nishantwrp Oct 31, 2020

aks681 approved these changes Oct 31, 2020

View reviewed changes

aks681 removed their assignment Oct 31, 2020

oppiabot bot added the PR: don't merge - HAS MERGE CONFLICTS label Nov 2, 2020

oppiabot bot assigned krishita30j Nov 2, 2020

Resolving conflicts

d79fbe2

oppiabot bot added PR: don't merge - HAS MERGE CONFLICTS and removed PR: don't merge - HAS MERGE CONFLICTS labels Nov 2, 2020

Resolving conflicts

0abb66f

oppiabot bot removed the PR: don't merge - HAS MERGE CONFLICTS label Nov 3, 2020

srijanreddy98 requested review from nithusha21 and removed request for srijanreddy98 November 4, 2020 12:12

srijanreddy98 assigned nithusha21 and unassigned srijanreddy98 Nov 4, 2020

nithusha21 approved these changes Nov 5, 2020

View reviewed changes

oppiabot bot unassigned nithusha21 Nov 5, 2020

oppiabot bot added PR: LGTM PR: don't merge - HAS MERGE CONFLICTS labels Nov 5, 2020

Resolving conflict

0476c0a

oppiabot bot removed the PR: don't merge - HAS MERGE CONFLICTS label Nov 9, 2020

kevintab95 merged commit 8d49eb4 into oppia:develop Nov 9, 2020

krishita30j deleted the typescriptCheck branch November 10, 2020 06:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix part of #10474: Cover more files with strict checks #11100

Fix part of #10474: Cover more files with strict checks #11100

krishita30j commented Oct 31, 2020 •

edited

vojtechjelinek left a comment

vojtechjelinek Oct 31, 2020

vojtechjelinek Oct 31, 2020

vojtechjelinek Oct 31, 2020

oppiabot bot commented Oct 31, 2020

krishita30j commented Oct 31, 2020

vojtechjelinek commented Oct 31, 2020

vojtechjelinek left a comment

nishantwrp left a comment

oppiabot bot commented Oct 31, 2020

aks681 left a comment

oppiabot bot commented Nov 2, 2020

oppiabot bot commented Nov 2, 2020

oppiabot bot commented Nov 5, 2020

oppiabot bot commented Nov 9, 2020

		// eslint-disable-next-line max-len
		existingNumSpaces = Object.keys(numSpacesToDesiredIndentLevel).map(Number);

		// eslint-disable-next-line max-len
		var existingNumSpaces = Object.keys(numSpacesToDesiredIndentLevel).map(Number);

	var numSpacesToDesiredIndentLevel: { [key: number]: number; } = {
	var numSpacesToDesiredIndentLevel: { [key: number]: number } = {

Fix part of #10474: Cover more files with strict checks #11100

Fix part of #10474: Cover more files with strict checks #11100

Conversation

krishita30j commented Oct 31, 2020 • edited

Overview

Essential Checklist

PR Pointers

vojtechjelinek left a comment

Choose a reason for hiding this comment

vojtechjelinek Oct 31, 2020

Choose a reason for hiding this comment

vojtechjelinek Oct 31, 2020

Choose a reason for hiding this comment

vojtechjelinek Oct 31, 2020

Choose a reason for hiding this comment

oppiabot bot commented Oct 31, 2020

krishita30j commented Oct 31, 2020

vojtechjelinek commented Oct 31, 2020

vojtechjelinek left a comment

Choose a reason for hiding this comment

nishantwrp left a comment

Choose a reason for hiding this comment

oppiabot bot commented Oct 31, 2020

aks681 left a comment

Choose a reason for hiding this comment

oppiabot bot commented Nov 2, 2020

oppiabot bot commented Nov 2, 2020

oppiabot bot commented Nov 5, 2020

oppiabot bot commented Nov 9, 2020

krishita30j commented Oct 31, 2020 •

edited