Add Promesa to simplify working with promises #18767
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jenkins BuildsClick to see older builds (12)
|
clauxx
requested review from
ilmotta,
alwx,
cammellos,
FFFra,
flexsurfer,
briansztamfater,
ajayesivan,
J-Son89,
BalogunofAfrica,
ulisesmac,
ibrkhalil and
a team
siddarthkay
approved these changes
Feb 9, 2024
shivekkhurana
approved these changes
Feb 9, 2024
J-Son89
approved these changes
Feb 9, 2024
@shivekkhurana when/if merged, will make an issue to collect & track promises that are hidden behind callbacks so that anybody could get some practice using it. |
smohamedjavid
approved these changes
Feb 9, 2024
ulisesmac
reviewed
Feb 12, 2024
briansztamfater
approved these changes
Feb 14, 2024
ilmotta
approved these changes
Feb 16, 2024
There was a problem hiding this comment.
Thank you @clauxx. I remember two other external contributors also asked if promesa was available or if they should add it.
Let's keep an eye on guidelines too. I see you changed from .then to p/then, and .catch to p/catch. Consistency is important, so if we are going in that direction, maybe soon we should have a guideline specifying to prefer to not use promises with interop. p/then for instance automatically promisifies code, which is handy.
ulisesmac
approved these changes
Feb 19, 2024
ajayesivan
approved these changes
Feb 20, 2024
88% of end-end tests have passedFailed tests (5)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (42)Click to expandClass TestDeepLinksOneDevice:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
90% of end-end tests have passedFailed tests (4)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (43)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestDeepLinksOneDevice:
Class TestActivityMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
|
7 tasks
pavloburykh
force-pushed
the
cl-add-promesa-lib
branch
from
2c4cbbe to
283e323
Compare
|
@clauxx thanks for the PR. Ready for merge. |
clauxx
force-pushed
the
cl-add-promesa-lib
branch
from
283e323 to
64db69b
Compare
Signed-off-by: Cristian Lungu <lungucristian95@gmail.com>
Signed-off-by: Cristian Lungu <lungucristian95@gmail.com>
clauxx
force-pushed
the
cl-add-promesa-lib
branch
from
64db69b to
eed0a88
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #18758
Summary
This PR adds a new tool for working seamlessly with promises in clj/cljs, named promesa. As this is a follow-up to the promesa proposal/discussion PR, which describes the motivation behind this PR, I'm looking to get it in as it got some positive feedback and interest from several CCs. If merged, I will create an issue to track the migration to promesa, as it can be done incrementally.
To summarise, here's some of
promesa's functionality:p/then,p/->,p/->>);async/await- (p/let);Promise.all- (p/all,p/plet);To read more on it, check out the introduction to working with promesa. Most of its syntax shouldn't be challenging to devs already working with clojure, as it uses familiar patterns:
let,->,->>,(.then), etc.Aside from adding the
promesadependency, I added a simple example to show how it's used. For a more complex example that shows the value of this addition, checkout the original promesa discussion PR.In case you have questions/hesitations, please leave a review, I'll be happy to discuss.
Review notes
Testing notes
This PR touches the chat photo selection code, so we should make that photo selection and displaying them in the chat works as before. I tested it, but who knows.
Platforms
Areas that maybe impacted
Functional
Steps to test
status: ready