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

Jan 2025 Improvements #97

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Jan 2025 Improvements #97

wants to merge 4 commits into from

Conversation

andydlindsay
Copy link
Contributor

@andydlindsay andydlindsay commented Jan 9, 2025

Changes:

  • Change date update functionality to only change the in-memory db and not write back to the file (this will save students the confusion of seeing updates on the server that they didn't make)
  • Change all functions to arrow functions for consistency
  • Remove unnecessary dependencies (MD5 and body-parser)
  • Update formatting throughout the project for consistency
  • Move data-only endpoints to use the prefix /api
  • Return the newly created tweet in response to the POST request
  • Update node package versions
  • Package-lock file created with Node v16
  • Tested with Node versions 14, 16, and 18

@andydlindsay andydlindsay added enhancement dependencies Pull requests that update a dependency file labels Jan 9, 2025
@andydlindsay andydlindsay self-assigned this Jan 9, 2025
@ChristianNally
Copy link

For some reason I'm still permitted to see these PRs and could accept it.

Maybe my account needs to be removed from some authorization?

Happy New Year!

Choose a reason for hiding this comment

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

While we're modifying those, should we prefix the tweet routes with /api to be consistent with what we tell them in the future? Also, we should modify the POST route so it returns the tweet obj, instead of making them refetch the entire tweets[] for good practice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • I like moving to /api for consistency.
  • We have an assignment Refetch Tweets on Submission that would have to be updated as well if we change what the server returns.

Choose a reason for hiding this comment

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

Yeah the activity would need to match, but doesn't seem like too much of an update no?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants