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

2 Node 2 Upgrade #375

Open
wants to merge 14 commits into
base: maybe-the-real-node-is-the-friends-we-made-along-the-way
Choose a base branch
from

Conversation

skalnik
Copy link
Member

@skalnik skalnik commented May 23, 2023

How hard could this be? Probably very. But I can put forth some effort I suppose.

Targets #319

So far:

  • Bumped a few deps a bit more since they seemed Unhappy
  • Handpatched curly to work with newer Node. This is a dependency of node-s3, so a better path would be to replace that entirely.
  • Fucked with tests. Why does it async different now? No idea. But they're green! ✅
    • My current waitForReply approach is hella sus and I don't even think fully resolving this, so I need to revisit this.
    • Most tests work properly now using co as per examples in hubot-test-helper. Still need to get the voting timeout test to pass though.
    • All tests are now green ✅
  • AYP needs to be manually tested. Update: Oh hey that works!
    • A manual test of the FULL thing including s3 upload would be good, but it works in DEBUG=1 mode

Primarily `process.EventEmitter.prototype` doesn't work and we need to
`require('EventEmitter')` instead. So this line has been adjusted:
https://github.com/mafintosh/curly/blob/568c6359906dd26d58f69c4dde8c327d0b56f4ac/index.js#L51
Pulled from hubot-test-helper examples
The voting timeout test still fails 😕
@randomPoison

This comment was marked as spam.

@skalnik
Copy link
Member Author

skalnik commented May 25, 2023

Confirmation AYP seems to work locally:
ayp-DEBUG-1685056222520

You can see me fuckin with voting to manually boop some shit.

@skalnik skalnik marked this pull request as ready for review May 25, 2023 23:11
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