Skip to content
This repository has been archived by the owner on Feb 15, 2023. It is now read-only.

Make sid non optional in memberdto #40

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

thewrongjames
Copy link
Member

I was sure that I tried this the first time I did it, but I guess I didn't as it seems to work fine.

I also updated a bunch of dependencies whilst I was here, and made one change that they required.

An update to querystring meant that its values could now be arrays of strings as well, which meant I needed to check for that alongside the existing check for undefined where one such value was being used. The token being set to an array should be wrong anyway, so my change should, I believe, be correct.

Manually testing these was probably overkill, but I wanted to be confident I wasn't going to break anything, so I did test this with the join form and I tried to test it with the events app. I wasn't able to get the events app running locally. I ran into some error with dependencies as they were, and trying to update them without actually changing the project turned out to be something of a nightmare, given that there are packages in there with newest versions that depend on webpack 3 (and should really be replaced with newer alternatives)... Anyway, after a few hours of that I decided that as it wasn't really my job it wasn't worth it for a single character change. I'm pretty sure this shouldn't break anything, especially as the events app will only be reading members.

Also, this fails to build on github actions. I can't work out why. I've spent hours going down various rabbit holes introduced by what should have just been removing a single question mark... maybe that is all this pull request should have been. Apparently rolling back to nodemon 1.19.something will fix the failures on github actions, but before this PR we had a version newer than that, and that version introduces vulnerabilities. I'm ah... done for now, but if anyone else wants to take this further feel free.

I was sure that I tried this the first time I did it, but I guess I
didn't as it seems to work fine.

I also updated a bunch of dependencies whilst I was here, and made one
change that they required.

An update to querystring meant that its values could now be arrays of
strings as well, which meant I needed to check for that alongside the
existing check for undefined where one such value was being used. The
token being set to an array should be wrong anyway, so my change should,
I believe, be correct.

Manually testing these was probably overkill, but I wanted to be
confident I wasn't going to break anything, so I did test this with the
join form and I tried to test it with the events app. I wasn't able to
get the events app running locally. I ran into some error with
dependencies as they were, and trying to update them without actually
changing the project turned out to be something of a nightmare, given
that there are packages in there with newest versions that depend on
webpack 3 (and should really be replaced with newer alternatives)...
Anyway, after a few hours of that I decided that as it wasn't really my
job it wasn't worth it for a single character change. I'm _pretty_ sure
this shouldn't break anything, especially as the events app will only be
reading members.
Apparently it is probably the thing messing with github actions... I may
end up needing to downgrade it instead.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant