Issue 2387 #2503

LuigiZaccagnini · 2021-11-20T22:04:01Z

Fixes #2387

Type of Change

Bugfix: Change which fixes an issue
New Feature: Change which adds functionality
Documentation Update: Change which improves documentation
UI: Change which improves UI

Description

Updated the root package.json packages.
Added a global textEncoder for upgrade jsdom

Checklist

Quality: This PR builds and passes our npm test and works locally
Tests: This PR includes thorough tests or an explanation of why it does not
Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not (if applicable)
Documentation: This PR includes updated/added documentation to user exposed functionality or configuration variables are added/changed or an explanation of why it does not(if applicable)

gitpod-io · 2021-11-20T22:04:04Z

humphd

Looks good, tests are all passing. One small fix.

package.json

humphd · 2021-11-20T23:23:11Z

package.json

-    "@wordpress/wordcount": "2.15.0",
+    "@elastic/elasticsearch": "7.15.0",
+    "@elastic/elasticsearch-mock": "0.3.1",
+    "@wordpress/wordcount": "3.2.3",
    "body-parser": "1.19.0",


Filed #2504 to remove this. We don't need it.

You can't remove this withing fixing the other things I mention in #2504. Please add body-parser back, and read review comments more carefully.

package.json

src/api/status/public/index.html

src/backend/data/stats.js

Qiwen-Yu · 2021-11-22T08:14:09Z

Mark this one as the code review template and example to me.

src/backend/data/stats.js

manekenpix

Please don't merge master into your branch. Revert that change and rebase.

manekenpix

I'll block this until the PR gets rebased.

humphd · 2021-11-28T20:48:28Z

Can we get another review on this?

humphd · 2021-11-28T21:53:07Z

@LuigiZaccagnini this needs another rebase:

git checkout master
git pull upstream master
git checkout issue-2387
git rebase master
# fix merge conflict in package.json
git push origin issue-2387 -f

Then ping me so I can review and merge.

humphd · 2021-12-03T18:22:49Z

What's happening with this?

humphd · 2021-12-03T20:32:14Z

This is out of date with package.json, please fix.

outdated normalize url, Bull and JSDom Updated jsdom and fixed ReferenceError: TextEncoder is not defined Upgraded Bull package Prettier files Removed body-parser and wordcount from package.json Removed wordcount Prettier index Added wordcount back to stats.js Added wordcount module Updated worcount

humphd

I don't understand the changes to the non-package.json files in this. Why would prettier change things?

I think the rest is good. I'll add @menghif as a reviewer, since this will cause churn in #2545

LuigiZaccagnini · 2021-12-03T22:50:19Z

The change in src/backend/utils/html/dom.js was done because the updates in the package.json required an encoder for JSDom. The other changes were prettier as they weren't passing the tests on my end with the rebase.

humphd · 2021-12-03T22:51:07Z

OK, sounds good.

menghif

Looks good! This will impact #2563 but seems like no one is working on it yet.

humphd · 2021-12-04T16:36:16Z

OK, going to merge this. FYI @HyperTHD, @manekenpix, in case this breaks staging in ways we haven't tested.

vercel bot deployed to Preview November 20, 2021 22:04 View deployment

LuigiZaccagnini requested a review from humphd November 20, 2021 22:05

vercel bot deployed to Preview November 20, 2021 22:19 View deployment

humphd requested changes Nov 20, 2021

View reviewed changes

vercel bot deployed to Preview November 21, 2021 04:37 View deployment

vercel bot deployed to Preview November 22, 2021 02:41 View deployment

humphd requested changes Nov 22, 2021

View reviewed changes

package.json Show resolved Hide resolved

src/api/status/public/index.html Show resolved Hide resolved

src/backend/data/stats.js Outdated Show resolved Hide resolved

LuigiZaccagnini force-pushed the issue-2387 branch from 357a496 to 7acfca4 Compare November 22, 2021 03:21

vercel bot deployed to Preview November 22, 2021 03:21 View deployment

humphd requested changes Nov 22, 2021

View reviewed changes

src/backend/data/stats.js Outdated Show resolved Hide resolved

vercel bot deployed to Preview November 22, 2021 14:39 View deployment

vercel bot deployed to Preview November 22, 2021 14:56 View deployment

vercel bot deployed to Preview November 22, 2021 15:11 View deployment

LuigiZaccagnini force-pushed the issue-2387 branch from 704d037 to cbe090b Compare November 22, 2021 15:21

vercel bot deployed to Preview November 22, 2021 15:21 View deployment

humphd previously approved these changes Nov 22, 2021

View reviewed changes

humphd requested a review from Qiwen-Yu November 22, 2021 16:07

humphd mentioned this pull request Nov 22, 2021

chore: bump passport-saml from 3.1.2 to 3.2.0 #2509

Closed

vercel bot deployed to Preview November 22, 2021 23:33 View deployment

manekenpix reviewed Nov 23, 2021

View reviewed changes

manekenpix requested changes Nov 23, 2021

View reviewed changes

LuigiZaccagnini dismissed humphd’s stale review via c1a198d November 23, 2021 15:58

LuigiZaccagnini force-pushed the issue-2387 branch from 446c663 to c1a198d Compare November 23, 2021 15:58

vercel bot deployed to Preview November 23, 2021 15:58 View deployment

LuigiZaccagnini force-pushed the issue-2387 branch from c1a198d to d0c91bf Compare November 23, 2021 16:24

vercel bot deployed to Preview November 23, 2021 16:24 View deployment

LuigiZaccagnini requested review from manekenpix and humphd November 23, 2021 16:26

humphd previously approved these changes Nov 23, 2021

View reviewed changes

manekenpix previously approved these changes Nov 28, 2021

View reviewed changes

LuigiZaccagnini dismissed stale reviews from manekenpix and humphd via ae642b7 November 30, 2021 15:39

LuigiZaccagnini force-pushed the issue-2387 branch from d0c91bf to ae642b7 Compare November 30, 2021 15:39

vercel bot deployed to Preview November 30, 2021 15:39 View deployment

LuigiZaccagnini force-pushed the issue-2387 branch from ae642b7 to d0c91bf Compare December 3, 2021 19:28

LuigiZaccagnini force-pushed the issue-2387 branch from d0c91bf to 5ac485a Compare December 3, 2021 22:33

vercel bot deployed to Preview December 3, 2021 22:33 View deployment

LuigiZaccagnini requested review from humphd and manekenpix December 3, 2021 22:42

humphd approved these changes Dec 3, 2021

View reviewed changes

humphd requested a review from menghif December 3, 2021 22:47

menghif approved these changes Dec 4, 2021

View reviewed changes

humphd merged commit 3b8f365 into Seneca-CDOT:master Dec 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue 2387 #2503

Issue 2387 #2503

LuigiZaccagnini commented Nov 20, 2021

gitpod-io bot commented Nov 20, 2021

humphd left a comment

humphd Nov 20, 2021

humphd Nov 21, 2021

Qiwen-Yu commented Nov 22, 2021

manekenpix left a comment

manekenpix left a comment

humphd commented Nov 28, 2021

humphd commented Nov 28, 2021

humphd commented Dec 3, 2021

humphd commented Dec 3, 2021

humphd left a comment

LuigiZaccagnini commented Dec 3, 2021

humphd commented Dec 3, 2021

menghif left a comment

humphd commented Dec 4, 2021

Issue 2387 #2503

Issue 2387 #2503

Conversation

LuigiZaccagnini commented Nov 20, 2021

Fixes #2387

Type of Change

Description

Checklist

gitpod-io bot commented Nov 20, 2021

humphd left a comment

Choose a reason for hiding this comment

humphd Nov 20, 2021

Choose a reason for hiding this comment

humphd Nov 21, 2021

Choose a reason for hiding this comment

Qiwen-Yu commented Nov 22, 2021

manekenpix left a comment

Choose a reason for hiding this comment

manekenpix left a comment

Choose a reason for hiding this comment

humphd commented Nov 28, 2021

humphd commented Nov 28, 2021

humphd commented Dec 3, 2021

humphd commented Dec 3, 2021

humphd left a comment

Choose a reason for hiding this comment

LuigiZaccagnini commented Dec 3, 2021

humphd commented Dec 3, 2021

menghif left a comment

Choose a reason for hiding this comment

humphd commented Dec 4, 2021