Skip to content

Update to be compatible with Node 16 - #40

Closed
0xf0xx0 wants to merge 9 commits into
discord:masterfrom
0xf0xx0:master
Closed

Update to be compatible with Node 16#40
0xf0xx0 wants to merge 9 commits into
discord:masterfrom
0xf0xx0:master

Conversation

@0xf0xx0

@0xf0xx0 0xf0xx0 commented Aug 3, 2021

Copy link
Copy Markdown

Ended up being three lines to remove.

Done on Linux 5.13.7-arch1-1 #1 SMP PREEMPT Sat, 31 Jul 2021 13:18:52 +0000 x86_64 GNU/Linux

@ghost ghost mentioned this pull request Aug 6, 2021
@fredkilbourn

Copy link
Copy Markdown

Looking forward to seeing this merged, nice work!

@Milo123459

Copy link
Copy Markdown

Vital for DJS' V13! Great work

@DraftProducts

Copy link
Copy Markdown

Is there an approximative date for the merge?

@0xf0xx0

0xf0xx0 commented Aug 8, 2021

Copy link
Copy Markdown
Author

@DraftProducts Most likely not since discord hasn't looked at this repo in a year, I'm thinkin of just making my fork the "main" active repo

@Milo123459

Copy link
Copy Markdown

@DraftProducts Most likely not since discord hasn't looked at this repo in a year, I'm thinkin of just making my fork the "main" active repo

Please do! Maybe publish it as an npm package rather than having to install via git!

@DraftProducts

Copy link
Copy Markdown

Maybe a ping to jasoncitron ? He is the biggest contributor🤔

@nemoralis

Copy link
Copy Markdown

You can install GingkathFox's repo with npm i GingkathFox/erlpack

@0xf0xx0

0xf0xx0 commented Aug 9, 2021

Copy link
Copy Markdown
Author

@DraftProducts Most likely not since discord hasn't looked at this repo in a year, I'm thinkin of just making my fork the "main" active repo

Please do! Maybe publish it as an npm package rather than having to install via git!

@Milo123459 you can npm i GingkathFox/erlpack

@fredkilbourn

Copy link
Copy Markdown

Maybe open an issue on discord repo and reference this?

@JasonDevTech

Copy link
Copy Markdown

Hi guys, I've also wanted to know the status of this but I got a bit bored of waiting and decided to explore it on my own. I rewrote it to Typescript with browser support. Here's the repo if you guys want to check it out.

@DraftProducts

Copy link
Copy Markdown

Hi guys, I've also wanted to know the status of this but I got a bit bored of waiting and decided to explore it on my own. I rewrote it to Typescript with browser support. Here's the repo if you guys want to check it out.

It's a nice project, but I don't think that your package will give the same perfs as the official package with bindings 🤔

@Milo123459

Copy link
Copy Markdown

@jasoncitron could you take a look at this?

@fredkilbourn

Copy link
Copy Markdown

@jhgg @adill @zorkian @mezz @fozzle @samschlegel @night - tagging some more discord people that are contributors, this is blocking full optimization with the new discord.js v13 library, which requires node v16, which requires this pull (or similar) to work.

@0xf0xx0

0xf0xx0 commented Aug 12, 2021

Copy link
Copy Markdown
Author

Merged #41 with this one since @Hazmi35 also updated deps

@DraftProducts

DraftProducts commented Aug 13, 2021

Copy link
Copy Markdown

We are now waiting about 10 days for this merge, I would make a fork of this repo and merge this PR myself.
Thanks @GingkathFox for the work 👍

@0xf0xx0

0xf0xx0 commented Aug 13, 2021

Copy link
Copy Markdown
Author

Oh wow it's been 10 days already

Ig move to my fork?

@0xf0xx0

0xf0xx0 commented Aug 13, 2021

Copy link
Copy Markdown
Author

I've put in a ticket to detatch my fork from the main repo

@Milo123459

Copy link
Copy Markdown

Your fork / published package doesn't work with DJS as it simply won't pick up your erlpack package.

@Milo123459

Copy link
Copy Markdown

Also I think it's a bit scummy to make a PR changing who maintains the repo. Also remove the pnpm lock

@icdevin

icdevin commented Aug 13, 2021

Copy link
Copy Markdown

This has no chance of being accepted as it exists now since it tells users to install his own repo and changes the name and author/maintainers...if you're going to spin off your own, do so and feel free tell people but it would've been wiser to leave this PR with just the changes needed for Node v16.

@Milo123459

Copy link
Copy Markdown

At this rate, either fix the PR with only needed changes, or someone make a new PR. It just makes no sense, changing the author, name, readme, etc to the name of your fork then attempting to make that change in the main repo? Doesn't make sense to me. Please revert to just needed changes

@fredkilbourn

Copy link
Copy Markdown

To be honest the best solution would be discord.js forking and maintaining this and not leaving users to fight for a fix. I've added an issue over there making the suggestion: discordjs/discord.js#6418 upvote over there if you agree.

@jhgg

jhgg commented Aug 13, 2021

Copy link
Copy Markdown
Contributor

Well you took a mergeable PR and made it totally unmergable. Feel free to re open with the original change set.

@jhgg jhgg closed this Aug 13, 2021
@Milo123459

Copy link
Copy Markdown

Oh for gods sake :(

@0xf0xx0

0xf0xx0 commented Aug 14, 2021

Copy link
Copy Markdown
Author

Oh whoops, I'll re-push with the proper fixes, didn't realize that would affect it

I like how this happens after everything lol

@Milo123459

Copy link
Copy Markdown

Oh whoops, I'll re-push with the proper fixes, didn't realize that would affect it

I like how this happens after everything lol

Not trying to be rude but how could you not see how this wouldn't affect it?

  • Change Authors
  • Renamed
  • Add a pnpm-lock (unneeded)
  • Then proceeded to make the main repo look like the fork?

@0xf0xx0

0xf0xx0 commented Aug 14, 2021

Copy link
Copy Markdown
Author

Oh whoops, I'll re-push with the proper fixes, didn't realize that would affect it

I like how this happens after everything lol

Not trying to be rude but how could you not see how this wouldn't affect it?

  • Change Authors

  • Renamed

  • Add a pnpm-lock (unneeded)

  • Then proceeded to make the main repo look like the fork?

I didn't realize my fork was edited on the master, I thought it had created a new branch with the fixes

Also the pnpm lock should've been ignored, I use pnpm instead of npm

@0xf0xx0 0xf0xx0 mentioned this pull request Aug 15, 2021
@Milo123459

Copy link
Copy Markdown

Pnpm lock should still be ignored

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.

9 participants