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

C2LC-575: Update react-scripts to version 5 #317

Merged
merged 6 commits into from
Jan 24, 2022
Merged

C2LC-575: Update react-scripts to version 5 #317

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6fbd540
C2LC-575: Update react-scripts to version 5.0.0
sbates-idrc Dec 21, 2021
7f24b42
C2LC-575: Disable flow-coverage-report
sbates-idrc Dec 21, 2021
8ab5003
C2LC-575: Update node-sass to 7.0.0
sbates-idrc Dec 21, 2021
ba470c9
C2LC-575: Remove flow-coverage-report
sbates-idrc Jan 6, 2022
eab585e
C2LC-575: Update node-sass to 7.0.1
sbates-idrc Jan 12, 2022
fbd2b4e
C2LC-575: Disable ESLint "no-use-before-define"
sbates-idrc Jan 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 0 additions & 14 deletions .flowcoverage-all
View file
Open in desktop

This file was deleted.

15 changes: 0 additions & 15 deletions .flowcoverage-main-threshold
View file
Open in desktop

This file was deleted.

13 changes: 0 additions & 13 deletions .flowcoverage-tests-threshold
View file
Open in desktop

This file was deleted.

14 changes: 1 addition & 13 deletions .github/workflows/ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-18.04]
node-version: [12.x, 14.x]
node-version: [14.x, 16.x]
runs-on: ${{ matrix.os }}
env:
CI: true
Expand Down Expand Up @@ -37,14 +37,6 @@ jobs:
run: npm run build
- name: Flow
run: npx flow check
- name: Flow Type Coverage (HTML report)
run: npx flow-coverage-report --config .flowcoverage-all
- name: Archive Flow Type Coverage Report
continue-on-error: true
uses: actions/upload-artifact@v2
with:
name: Flow Type Coverage Report
path: flow-coverage/
- name: Test
run: npm test -- --verbose --coverage
- name: Archive Test Code Coverage Report
Expand All @@ -53,9 +45,5 @@ jobs:
with:
name: Test Code Coverage Report
path: coverage/
- name: Flow Type Coverage (main coverage threshold)
run: npx flow-coverage-report --config .flowcoverage-main-threshold
- name: Flow Type Coverage (test coverage threshold)
run: npx flow-coverage-report --config .flowcoverage-tests-threshold
- name: Lint Documentation
run: npm run markdownlint
2 changes: 1 addition & 1 deletion netlify.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
[build]
environment = { NODE_VERSION = "12" }
environment = { NODE_VERSION = "14" }
command = "npm run netlify-build"
publish = "build/"
13,570 changes: 4,091 additions & 9,479 deletions package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

5 changes: 2 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,17 @@
"classnames": "2.3.1",
"enzyme": "3.11.0",
"enzyme-adapter-react-16": "1.15.6",
"node-sass": "4.14.1",
"node-sass": "7.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't see that you had updated this already when I commented earlier and filed C2LC-578. If we're already updating, then we should go ahead and update to 7.0.1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Thanks.

"react": "16.14.0",
"react-bootstrap": "1.6.4",
"react-dom": "16.14.0",
"react-intl": "5.22.0",
"react-scripts": "4.0.3",
"react-scripts": "5.0.0",
"tone": "14.7.77",
"typeface-roboto": "1.1.13"
},
"devDependencies": {
"flow-bin": "0.142.0",
"flow-coverage-report": "0.8.0",
"markdownlint-cli2": "0.3.2"
},
"scripts": {
Expand Down
10 changes: 10 additions & 0 deletions src/CharacterState.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export default class CharacterState {
return true;
}

// eslint-disable-next-line no-use-before-define
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there any circumstances in which this check is helpful to us? If not, maybe we should disable the rule rather than adding all these comments.

Copy link
Contributor Author

@sbates-idrc sbates-idrc Jan 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

That's a good question. Disabling the rule, would certainly make the code cleaner by not requiring these comments.

I think the rule offers some value but I think not a huge amount. I'll disable the rule and remove the comments. We can always reconsider. But I think minimizing the number of ESLint-specific comments is worthwhile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've disabled the "no-use-before-define" rule

forward(distance: number, drawingEnabled: boolean): CharacterState {
const movementResult = this.calculateMove(distance, this.direction, drawingEnabled);
return new CharacterState(
Expand All @@ -71,6 +72,7 @@ export default class CharacterState {
);
}

// eslint-disable-next-line no-use-before-define
backward(distance: number, drawingEnabled: boolean): CharacterState {
const movementResult = this.calculateMove(distance,
C2lcMath.wrap(0, 8, this.direction + 4), drawingEnabled);
Expand All @@ -83,6 +85,7 @@ export default class CharacterState {
);
}

// eslint-disable-next-line no-use-before-define
turnLeft(amountEighthsOfTurn: number): CharacterState {
return new CharacterState(
this.xPos,
Expand All @@ -93,6 +96,7 @@ export default class CharacterState {
);
}

// eslint-disable-next-line no-use-before-define
turnRight(amountEighthsOfTurn: number): CharacterState {
return new CharacterState(
this.xPos,
Expand All @@ -103,6 +107,7 @@ export default class CharacterState {
);
}

// eslint-disable-next-line no-use-before-define
moveUpPosition(): CharacterState {
let yPos = 0;
if (this.sceneDimensions.getBoundsStateY(this.yPos - 1) === 'outOfBoundsBelow') {
Expand All @@ -119,6 +124,7 @@ export default class CharacterState {
);
}

// eslint-disable-next-line no-use-before-define
moveRightPosition(): CharacterState {
let xPos = 0;
if (this.sceneDimensions.getBoundsStateX(this.xPos + 1) === 'outOfBoundsAbove') {
Expand All @@ -135,6 +141,7 @@ export default class CharacterState {
);
}

// eslint-disable-next-line no-use-before-define
moveDownPosition(): CharacterState {
let yPos = 0;
if (this.sceneDimensions.getBoundsStateY(this.yPos + 1) === 'outOfBoundsAbove') {
Expand All @@ -151,6 +158,7 @@ export default class CharacterState {
);
}

// eslint-disable-next-line no-use-before-define
moveLeftPosition(): CharacterState {
let xPos = 0;
if (this.sceneDimensions.getBoundsStateY(this.xPos - 1) === 'outOfBoundsBelow') {
Expand All @@ -167,6 +175,7 @@ export default class CharacterState {
);
}

// eslint-disable-next-line no-use-before-define
changeXPosition(columnLabel: string): CharacterState {
let newXPos = this.xPos;
if (columnLabel <= String.fromCharCode(64 + this.sceneDimensions.getWidth()) && columnLabel >='A') {
Expand All @@ -183,6 +192,7 @@ export default class CharacterState {
);
}

// eslint-disable-next-line no-use-before-define
changeYPosition(rowLabel: number): CharacterState {
let newYPos = this.yPos;
if (rowLabel <= this.sceneDimensions.getHeight() && rowLabel >= 1) {
Expand Down
8 changes: 8 additions & 0 deletions src/ProgramSequence.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,28 +31,34 @@ export default class ProgramSequence {
return this.program[index];
}

// eslint-disable-next-line no-use-before-define
updateProgram(program: Program): ProgramSequence {
return new ProgramSequence(program, this.programCounter);
}

// eslint-disable-next-line no-use-before-define
updateProgramCounter(programCounter: number): ProgramSequence {
return new ProgramSequence(this.program, programCounter);
}

// eslint-disable-next-line no-use-before-define
updateProgramAndProgramCounter(program: Program, programCounter: number): ProgramSequence {
return new ProgramSequence(program, programCounter);
}

// eslint-disable-next-line no-use-before-define
incrementProgramCounter(): ProgramSequence {
return new ProgramSequence(this.program, this.programCounter + 1);
}

// eslint-disable-next-line no-use-before-define
overwriteStep(index: number, command: string): ProgramSequence {
const program = this.program.slice();
program[index] = command;
return this.updateProgram(program);
}

// eslint-disable-next-line no-use-before-define
insertStep(index: number, command: string): ProgramSequence {
const program = this.program.slice();
program.splice(index, 0, command);
Expand All @@ -63,6 +69,7 @@ export default class ProgramSequence {
}
}

// eslint-disable-next-line no-use-before-define
deleteStep(index: number): ProgramSequence {
const program = this.program.slice();
program.splice(index, 1);
Expand All @@ -73,6 +80,7 @@ export default class ProgramSequence {
}
}

// eslint-disable-next-line no-use-before-define
swapStep(indexFrom: number, indexTo: number): ProgramSequence {
const program = this.program.slice();
if (program[indexFrom] != null && program[indexTo] != null) {
Expand Down