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

Promises v3 Checklist #1149

Closed
7 tasks done
valzargaming opened this issue Jul 13, 2023 · 13 comments · Fixed by #1157
Closed
7 tasks done

Promises v3 Checklist #1149

valzargaming opened this issue Jul 13, 2023 · 13 comments · Fixed by #1157
Labels
dependencies Pull requests that update a dependency file enhancement
Milestone

Comments

@valzargaming
Copy link
Member

valzargaming commented Jul 13, 2023

Per https://github.com/reactphp/promise/releases/tag/v3.0.0

Irrelevant:

  • Existing instances of FulfilledPromise and RejectedPromise classes must be updated to use resolve() and reject() functions instead
  • Existing instances of LazyPromise should be removed
  • Any instances of some(), map(), and reduce() functions must be replaced with the any() or all() functions (I don't think we use these)

Breaking changes:

Non-breaking changes:

  • otherwise() and always() are deprecated, replaced by catch() and finally() (only instance I could find was Discord.php)
@valzargaming valzargaming added enhancement dependencies Pull requests that update a dependency file labels Jul 13, 2023
@key2peace
Copy link
Collaborator

All this should be in https://github.com/discord-php/DiscordPHP/tree/promise-v3 now

@valzargaming
Copy link
Member Author

ExtendedPromiseInterface extends PromiseInterface, and Promise v2 implements ExtendedPromiseInterface, so instead of replacing the existing typehinting to PromiseInterface we could just use React\Promise\Promise, which would overcome the hurdle and be better for futureproofing code. This seems to be the ReactPHP developers' intent as well, given they have made all classes final to encourage composition.

@shehi
Copy link
Contributor

shehi commented Jul 31, 2023

It's painful to read this @valzargaming :) Code-ify more please.

@valzargaming
Copy link
Member Author

It's painful to read this @valzargaming :) Code-ify more please.

This is already coded in the promises v3 branch as of today.

@2colours
Copy link

What's the state of this issue?

@valzargaming
Copy link
Member Author

valzargaming commented Nov 17, 2024 via email

@2colours
Copy link

What are the dependencies in question? I was very much under the impression that it's all DiscordPHP stuff. If there are such external dependencies at all, it would make sense to open issues for them at least for tracking.

@2colours
Copy link

2colours commented Nov 19, 2024

I couldn't resist going through the "bottleneck" dependencies:

  • discord-php/http (constraint: ^2.2)
    • this is under your control
  • react/datagram (constraint: ~2.1)
    • DiscordPHP directly depends on an outdated version (1.5) of this library
    • DiscordPHP itself should be updated to a more recent version that does support more recent versions of react/promise

That's all I can see with composer show --tree. It doesn't seem like there is any external blocker - in fact, the biggest blocker is the direct dependency on another outdated react package (react/datagram). Maybe the issue should be about upgrading react/datagram, then.

UPDATE:
Sorry, this was for v7. For v10, react/datagram is upgraded and the only problem is the direct dependency on v2 in your own package discord-php/http.

@valzargaming
Copy link
Member Author

wyrihaximus/react-cache-redis still relies on react/promise ^2.8 and is the biggest change to the v10 release.

@valzargaming
Copy link
Member Author

valzargaming commented Nov 19, 2024

UPDATE: Sorry, this was for v7. For v10, react/datagram is upgraded and the only problem is the direct dependency on v2 in your own package discord-php/http.

@2colours, thanks for checking and confirming this! I announced on 9/25/2024 in our Discord that we have officially discontinued v7 support specifically for reasons like this along with Discord having marked the v7 gateway as deprecated. We will not take into consideration any issues with that older version when working on this future update. The HTTP library has an active PR that is undergoing review. Once we've confirmed functionality we will add ^3.0 to composer.json.

@2colours
Copy link

wyrihaximus/react-cache-redis still relies on react/promise ^2.8 and is the biggest change to the v10 release.

The two most recent versions of that package do support v3 Promises (the latest version, 4.5.0 at this moment, only supports that).

But anyway, most importantly, this is just a dev dependency. A dev dependency really shouldn't poison downstream users; I don't know how Composer works under the hood but it rather sounds like their issue if this can happen at all.

I announced on 9/25/2024 in our Discord that we have officially discontinued v7 support specifically for reasons like this along with Discord having marked the v7 gateway as deprecated.

You mean, the server I was banned from while trying to clarify #1208, why it's a library issue and not a "you problem" to swallow promise rejections, for mere ego reasons in my opinion? That's too bad, especially that this is a general purpose PHP Discord, not just a Discord for your library. Give people power to see their true nature...

@valzargaming
Copy link
Member Author

valzargaming commented Nov 20, 2024

wyrihaximus/react-cache-redis still relies on react/promise ^2.8 and is the biggest change to the v10 release.
But anyway, most importantly, this is just a dev dependency. A dev dependency really shouldn't poison downstream users; I don't know how Composer works under the hood but it rather sounds like their issue if this can happen at all.

You seem to have misunderstood the context here. The inclusion of the experimental interface is intentional and serves to notify developers of its experimental nature. This is why it’s currently restricted to dev usage, it’s not yet considered stable. While we originally planned to launch the stable version with v10, we chose to delay it to ensure its readiness.

You mean, the server I was banned from while trying to clarify #1208, why it's a library issue and not a "you problem" to swallow promise rejections, for mere ego reasons in my opinion? That's too bad, especially that this is a general purpose PHP Discord, not just a Discord for your library. Give people power to see their true nature...

This issue appears to be more about individual circumstances than the library itself. If you’d like to revisit the situation, you’re welcome to submit an unban request to maintainers@discordphp.org for review and explain why you were banned and why it should be lifted. Be sure to include your discord username or id.

Regarding your concerns, the purpose of the Discord server is to facilitate constructive discussions about this library and related topics. While it is a general-purpose PHP Discord, specific communities (like ours) often set their own guidelines and moderation standards. The decision to enforce a ban was not personal but based on the observed tone and interactions. Constructive dialogue is always encouraged, but communication should remain respectful and collaborative. GitHub's issue tracker remains open for technical discussions about the library itself, provided the conversation stays professional and focused on solutions. However, I will warn you that you should adjust your tone going forward. In this thread alone you have made ad hominem attacks and made statements that escalate conflict rather than facilitate resolution (e.g. Give people power to see their true nature...). Such behavior will not be tolerated within the Discord server.

@valzargaming
Copy link
Member Author

All necessary changes have been completed and have been tested in a production environment. React/Promise ^3 is now live on dev-master for testing. Closing this issue for now, please open a new issue report if any problems are found.

@valzargaming valzargaming added this to the 10.0 milestone Nov 20, 2024
@valzargaming valzargaming linked a pull request Nov 20, 2024 that will close this issue
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 a pull request may close this issue.

4 participants