Skip to content

Conversation

@humble-barnacle001
Copy link
Contributor

@humble-barnacle001 humble-barnacle001 commented Feb 11, 2024

Hi,
I wanted to improve certain things and make some bugfixes:

  • use a package instead of node_modules folder as per GitHub recommendations
  • upgrade to node 20 runner from node 16 to resolve actions warning
  • fix a typo in variable in index.js
  • use .env file instead of modifying test_config

@humble-barnacle001 humble-barnacle001 changed the title Update to node20 and use package instead of node_modules Update to node20 and use package instead of node_modules & bugfixes Feb 11, 2024
Copy link
Owner

@joshcai joshcai left a comment

Choose a reason for hiding this comment

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

Thank you @humble-barnacle001 - these improvements are great! I didn't know the recommendation to commit the node_modules has changed, I'm glad we no longer have to do that.

COMMIT_HEADER: 'Sync LeetCode submission'
FILTER_DUPLICATE_SECS: process.env.FILTER_DUPLICATE_SECS || 86400,
DESTINATION_FOLDER: process.env.DESTINATION_FOLDER || '',
VERBOSE: process.env.VERBOSE || true,
Copy link
Owner

Choose a reason for hiding this comment

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

I believe if process.env.VERBOSE === false, this would still turn into true with ||, I think ?? would work better for all of these. I'll change it on my side since I'm about to update the code a bit and don't want you to have to deal with the merge conflicts.

@joshcai joshcai merged commit 07e7b1b into joshcai:master Mar 25, 2024
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