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

housekeeping #1622

Merged
merged 19 commits into from
Feb 17, 2023
Merged

housekeeping #1622

merged 19 commits into from
Feb 17, 2023

Conversation

ballPointPenguin
Copy link
Contributor

@ballPointPenguin ballPointPenguin commented Feb 13, 2023 •
edited
Loading

These are a handful of small commits that I cherry-picked from a much larger #1617 . Small things fixed along the way.
They are better off treated in their own smaller PR here and are not fundamentally part of the configuration work.

One fun addition is actual jest testing in the "server" app. Just a couple little tests to start with but they can be a good sanity check while you work (esp. with VS Code Jest plugin re-running them on change)

image

Reviewer Note Please read through the commit messages below for a list of changes in this mixed bag PR. 👇🏻

xeeg
xeeg reviewed Feb 15, 2023
View reviewed changes
Copy link
Contributor

@xeeg xeeg left a comment •
edited
Loading

Choose a reason for hiding this comment

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

It would be great to add a comment to the "Testing" section on README about testing with jest

@ballPointPenguin
Copy link
Contributor Author

It would be great to add a comment to the "Testing" section on README about testing with jest

Yes! I'm including that at the bottom of the README rewrite in #1617
https://github.com/compdemocracy/polis/blob/configuration-unification/server/README.md

metasoarous
metasoarous reviewed Feb 17, 2023
View reviewed changes
server/app.ts
@@ -113,8 +113,8 @@ helpersInitialized.then(
handle_GET_snapshot,
handle_GET_stripe_account_connect,
handle_GET_stripe_account_connected_oauth_callback,
hangle_GET_testConnection,
Copy link
Member

Choose a reason for hiding this comment

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

LOL; It took me like a solid minute to catch the diff

metasoarous
metasoarous reviewed Feb 17, 2023
View reviewed changes
server/src/server.ts
shouldCreateXidRecord = xidUser === "noXidRecord";
if (xidUser && xidUser.uid) {
Copy link
Member

Choose a reason for hiding this comment

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

Is there any other scenario where we wouldn't have a uid, or is that enforced by the type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Typescript caught this for me. this xidUser variable can be either a UserType object or the string "noXidRecord". The type is checked as UserType which will have a uid property.

@metasoarous metasoarous merged commit e5f19bb into edge Feb 17, 2023
@metasoarous metasoarous added this to the Hugo milestone Feb 17, 2023
ballPointPenguin pushed a commit that referenced this pull request Feb 26, 2023
@dependabot @ballPointPenguin
Merge branch 'edge' into colinmegill/client-report-package-upgrades
a5b7e07 
* edge: (22 commits)
  Configuration unification (#1617)
  housekeeping (#1622)
  Remove intercom (#1547)
  Reduced bundlesize by requiring only victory-core for VictoryAnimation component. (#1618)
  A variety of improvements all around (#1608)
  Update client-participation dependencies, and get off gulp -> webpack (#1607)
  Feature/crkrenn/fixed participants extended sql table (#1586)
  Switch server to debian (#1600)
  More slack removal (#1599)
  Update server node & misc dep versions (#1595)
  switch from npm ci to npm install in client dockerfiles (#1594)
  deleted docker_cloud_notes (#1597)
  Remove slack (#1585)
  specify bullseye debian base for math dockerfile (#1590)
  Feature/crkrenn/improved makefile (#1577)
  Adding direction to the client- participation (#1561)
  update winston package (#1584)
  add scaling roadmap doc (#1582)
  Feature/sirodoht crk/e2e version 11 (#1576)
  German language editing (#1552)
  ...
@ballPointPenguin ballPointPenguin deleted the br/housekeeping branch February 26, 2023 17:23
ballPointPenguin pushed a commit that referenced this pull request Mar 4, 2023
@dependabot @ballPointPenguin
Merge branch 'edge' into colinmegill/client-report-package-upgrades
7b21809 
* edge: (22 commits)
  Configuration unification (#1617)
  housekeeping (#1622)
  Remove intercom (#1547)
  Reduced bundlesize by requiring only victory-core for VictoryAnimation component. (#1618)
  A variety of improvements all around (#1608)
  Update client-participation dependencies, and get off gulp -> webpack (#1607)
  Feature/crkrenn/fixed participants extended sql table (#1586)
  Switch server to debian (#1600)
  More slack removal (#1599)
  Update server node & misc dep versions (#1595)
  switch from npm ci to npm install in client dockerfiles (#1594)
  deleted docker_cloud_notes (#1597)
  Remove slack (#1585)
  specify bullseye debian base for math dockerfile (#1590)
  Feature/crkrenn/improved makefile (#1577)
  Adding direction to the client- participation (#1561)
  update winston package (#1584)
  add scaling roadmap doc (#1582)
  Feature/sirodoht crk/e2e version 11 (#1576)
  German language editing (#1552)
  ...
ballPointPenguin pushed a commit that referenced this pull request Mar 5, 2023
@dependabot @ballPointPenguin
Merge branch 'edge' into colinmegill/client-report-package-upgrades
c834c6f 
* edge: (22 commits)
  Configuration unification (#1617)
  housekeeping (#1622)
  Remove intercom (#1547)
  Reduced bundlesize by requiring only victory-core for VictoryAnimation component. (#1618)
  A variety of improvements all around (#1608)
  Update client-participation dependencies, and get off gulp -> webpack (#1607)
  Feature/crkrenn/fixed participants extended sql table (#1586)
  Switch server to debian (#1600)
  More slack removal (#1599)
  Update server node & misc dep versions (#1595)
  switch from npm ci to npm install in client dockerfiles (#1594)
  deleted docker_cloud_notes (#1597)
  Remove slack (#1585)
  specify bullseye debian base for math dockerfile (#1590)
  Feature/crkrenn/improved makefile (#1577)
  Adding direction to the client- participation (#1561)
  update winston package (#1584)
  add scaling roadmap doc (#1582)
  Feature/sirodoht crk/e2e version 11 (#1576)
  German language editing (#1552)
  ...
ballPointPenguin pushed a commit that referenced this pull request Mar 6, 2023
@dependabot @ballPointPenguin
Merge branch 'edge' into colinmegill/client-report-package-upgrades
ecf0898 
* edge: (22 commits)
  Configuration unification (#1617)
  housekeeping (#1622)
  Remove intercom (#1547)
  Reduced bundlesize by requiring only victory-core for VictoryAnimation component. (#1618)
  A variety of improvements all around (#1608)
  Update client-participation dependencies, and get off gulp -> webpack (#1607)
  Feature/crkrenn/fixed participants extended sql table (#1586)
  Switch server to debian (#1600)
  More slack removal (#1599)
  Update server node & misc dep versions (#1595)
  switch from npm ci to npm install in client dockerfiles (#1594)
  deleted docker_cloud_notes (#1597)
  Remove slack (#1585)
  specify bullseye debian base for math dockerfile (#1590)
  Feature/crkrenn/improved makefile (#1577)
  Adding direction to the client- participation (#1561)
  update winston package (#1584)
  add scaling roadmap doc (#1582)
  Feature/sirodoht crk/e2e version 11 (#1576)
  German language editing (#1552)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xeeg xeeg xeeg left review comments

@metasoarous metasoarous metasoarous left review comments

@colinmegill colinmegill Awaiting requested review from colinmegill

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Hugo
Development

Successfully merging this pull request may close these issues.
3 participants
@ballPointPenguin @metasoarous @xeeg