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

Added an awaitable Promise wrapper for getCookies #21

Merged
merged 3 commits into from
May 2, 2021

Conversation

rubengmurray
Copy link
Collaborator

No description provided.

@rubengmurray rubengmurray changed the title added wrapper for promises Added an awaitable Promise wrapper for getCookies Mar 22, 2020
@textbook
Copy link

Note Node comes with an implementation of that wrapper, as I show in https://stackoverflow.com/a/65265955/3001761

@rubengmurray
Copy link
Collaborator Author

Note Node comes with an implementation of that wrapper, as I show in https://stackoverflow.com/a/65265955/3001761

Thanks for updating this PR with the link @textbook - I believe that wrapper will solve all cases except for where profile is required, as profile becomes the last argument in the function call.

Still, given this PR has been hanging around for 9 months, I'm sure your answer will help many.

@suntong
Copy link

suntong commented Mar 20, 2021

Now this PR has been about a year

@rubengmurray
Copy link
Collaborator Author

Now this PR has been about a year

Yeah this is no longer maintained by the owner unfortunately.

@suntong
Copy link

suntong commented Mar 22, 2021

Yeah, noticed that as well.

I found your PR merged in at https://github.com/paragbaxi/chrome-cookies-secure, FTR.

@rubengmurray
Copy link
Collaborator Author

Had no idea about that fork. Cheers

@paragbaxi
Copy link

paragbaxi commented Mar 22, 2021 via email

@rubengmurray
Copy link
Collaborator Author

Need any assistance? I'm pretty invested in this package and still use it everyday. Hadn't heard of playwright... until now. How are you going about raising?

@bertrandom
Copy link
Owner

Hi, sorry for the radio silence here - don't have a lot of great excuses but let's just say toddler + COVID has made me less attentive to my repos than I'd like to be. @rubengmurray would you be interested in taking this module over if you have the bandwidth for it?

@paragbaxi
Copy link

self/family first! i added you @rubengmurray as a contributor

@rubengmurray
Copy link
Collaborator Author

rubengmurray commented Mar 25, 2021

Yeah I'd be happy to assist with this going forward as a maintainer.

What's the best way to get the latest code changes on the existing npm package? It would be good for continuity if existing package users could benefit from updates and we can have a look through the open PRs here.

Can we merge paragbaxi/chrome-cookies-secure into here and @paragbaxi & I become maintainers?

@bertrandom
Copy link
Owner

@rubengmurray @paragbaxi Can you give me your npm usernames and I can add you as maintainers there and here and you can take it from there? Thanks.

@rubengmurray
Copy link
Collaborator Author

@rubengmurray @paragbaxi Can you give me your npm usernames and I can add you as maintainers there and here and you can take it from there? Thanks.

Same username on npm, rubengmurray

@paragbaxi
Copy link

@rubengmurray @paragbaxi Can you give me your npm usernames and I can add you as maintainers there and here and you can take it from there? Thanks.

paragbaxi

@paragbaxi
Copy link

Can we merge paragbaxi/chrome-cookies-secure into here and @paragbaxi & I become maintainers?

Makes sense.

@bertrandom
Copy link
Owner

@rubengmurray @paragbaxi Invites sent, please merge/modify in whatever way you see fit. Thanks! Let me know if there's anything else you need.

@rubengmurray
Copy link
Collaborator Author

rubengmurray commented Mar 26, 2021

@paragbaxi it looks like your version is just this repo with:

Perhaps it's just easier to merge these two PRs in here rather than merge repos or your master to this... if you're happy with that on reading this then please do merge 👍 and we'll crack on over the next few weeks reviewing the open PRs for this repo?

@rubengmurray
Copy link
Collaborator Author

@paragbaxi are you happy to merge / approve this in? I'm wary of doing it myself given it's my own work.

@paragbaxi
Copy link

My bad for the delay. Excited to see movement here. Yeah I can do it.

@paragbaxi
Copy link

noticed the line break. any style we're following? i prefer prettier default

@rubengmurray
Copy link
Collaborator Author

rubengmurray commented Apr 23, 2021

noticed the line break. any style we're following? i prefer prettier default

Prettier default is good with me 👍 good suggestion

Perhaps a separate PR with Prettier config applied to the entire repo is in order?

@rubengmurray rubengmurray merged commit 08bbbae into bertrandom:master May 2, 2021
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.

None yet

5 participants