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.

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

Miscellaneous JS Fixes #257

Merged
merged 4 commits into from
Aug 4, 2023
Merged

Conversation

alecpm
Copy link
Contributor

@alecpm alecpm commented Aug 4, 2023

Some JS cleanup. Add pre-commit hooks to validate YAML and run JS build.

@alecpm alecpm force-pushed the miscellaneous-js-build-cleanup branch from 92a30e2 to caf24d7 Compare August 4, 2023 18:59
@alecpm alecpm changed the base branch from issues/234-player-item-interaction to one-hour-one-life August 4, 2023 19:00
Copy link
Contributor

@jessesnyder jessesnyder left a comment

Choose a reason for hiding this comment

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

Ahhhh... Looks great.

I think _.isNil should be _.isNull unless there's something I didn't see in the docs.

@@ -1078,7 +1076,7 @@ function onGameStateChange(msg) {
updateDonationStatus(state.donation_active);

// Update gridItems
if (state.items !== undefined && state.items !== null) {
if (! _.isNil(state.items)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is isNull() ? https://underscorejs.org/#isNull

Copy link
Contributor Author

@alecpm alecpm Aug 4, 2023

Choose a reason for hiding this comment

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

We're using lodash, so it's isNil. There's also isNull in lodash, but this seems like a better fit here. isNull(x) is just x === null so I doubt we'd ever actually want to use it.

@@ -0,0 +1,4 @@
*.js -diff -merge
Copy link
Contributor

Choose a reason for hiding this comment

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

I must have just had the file in the wrong place(?) I had it at the repository root.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's OK to have in the repository root, but then the glob paths are more difficult I think,

…leanup miscellaneous JS issues and deprecations.
@alecpm alecpm force-pushed the miscellaneous-js-build-cleanup branch from caf24d7 to 554f3a3 Compare August 4, 2023 22:01
@alecpm alecpm merged commit 293390c into one-hour-one-life Aug 4, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants