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

Lot of mutateAsync functions not being awaited #2205

Closed
3 tasks done
harshsbhat opened this issue Oct 5, 2024 · 24 comments · Fixed by #2232
Closed
3 tasks done

Lot of mutateAsync functions not being awaited #2205

harshsbhat opened this issue Oct 5, 2024 · 24 comments · Fixed by #2232

Comments

@harshsbhat
Copy link
Contributor

Preliminary Checks

Reproduction / Replay Link (Optional)

No response

Issue Summary

It is usually a good practice to await the mutation async functions. We are missing that in a lot of instances. I have just listed one of them. There are at least a few more of these being used without awaiting

  async function onSubmit(values: z.infer<typeof formSchema>) {
    updateRatelimit.mutateAsync(values);
  }

Steps to Reproduce

NA

Expected behavior

We should await these functions.

Other information

No response

Screenshots

No response

Version info

- OS:
- Node:
- npm:
@harshsbhat harshsbhat added Bug Something isn't working Needs Approval Needs approval from Unkey labels Oct 5, 2024
Copy link

linear bot commented Oct 5, 2024

@chronark
Copy link
Collaborator

chronark commented Oct 5, 2024

/award 150 points

Copy link

oss-gg bot commented Oct 5, 2024

Awarding harshsbhat: 150 points 🕹️ Well done! Check out your new contribution on oss.gg/harshsbhat

@NeerajReddyK
Copy link

Can I get that assigned? (if it is up for grabs)

@NeerajReddyK
Copy link

/assign

Copy link

oss-gg bot commented Oct 5, 2024

Assigned to @NeerajReddyK! Please open a draft PR linking this issue within 48h ⚠️ If we can't detect a PR from you linking this issue in 48h, you'll be unassigned automatically 🕹️ Excited to have you ship this 🚀

@harshsbhat
Copy link
Contributor Author

/assign

I was testing it and was done with it. If you have started working I can discard the changes. But if you haven't can you stop?

@NeerajReddyK
Copy link

Sure. I request you to proceed

@NeerajReddyK NeerajReddyK removed their assignment Oct 5, 2024
@DeepaPrasanna
Copy link
Contributor

/assign

I was testing it and was done with it. If you have started working I can discard the changes. But if you haven't can you stop?

Hey, @chronark will I be wrong in saying that one should start working on an issue only after being assigned, so that others wont have any misunderstanding. I have seen in other oss.gg repos that you cant work on more than 1 issue in the same repo at the same time. You can move to another issue only after your previous issue has been merged/closed so that this gives everyone a fair chance. Apologies if I misunderstood/or spoke wrong 🙏

@harshsbhat
Copy link
Contributor Author

harshsbhat commented Oct 5, 2024

/assign

I was testing it and was done with it. If you have started working I can discard the changes. But if you haven't can you stop?

Hey, @chronark will I be wrong in saying that one should start working on an issue only after being assigned, so that others wont have any misunderstanding. I have seen in other oss.gg repos that you cant work on more than 1 issue in the same repo at the same time. You can move to another issue only after your previous issue has been merged/closed so that this gives everyone a fair chance. Apologies if I misunderstood/or spoke wrong 🙏

The thing is my other PR ( which I was assigned is already approved and tests are running ) that's why I asked him. Hope that was fair. I apologies if it was fair.

@DeepaPrasanna
Copy link
Contributor

I think you should wait for your PR to be closed/merged. You can try /assign here. I believe it wont work when you have open PRs

@chronark
Copy link
Collaborator

chronark commented Oct 5, 2024

Hmm, yeah we should try to distribute issues a bit better, so newcomers have a chance to grab some too.
The "only one active PR" rule is a bit too strict sometimes, cause that means you're potentially blocked on my review.

@DeepaPrasanna
Copy link
Contributor

Yes, I do agree. Yesterday, I had to wait until my PR got merged to assign myself another(I understand It is not easy you guys do reviews and work on new features at the same time. I am grateful for all the reviews that I recieve.) but I believe we can use that time to explore other repos or try non code contributions.

@harshsbhat
Copy link
Contributor Author

should I open the PR for this one or not then?

@chronark @DeepaPrasanna I think having a tag like first-timers only like freecodecamp can help us give new comers a good and fair chance on the simpler issues wdyt?

@chronark
Copy link
Collaborator

chronark commented Oct 5, 2024

let's leave it open for someone else to come in
if nobody takes it within the next 24h, go ahead

@geraldmaboshe
Copy link
Contributor

/assign

Copy link

oss-gg bot commented Oct 5, 2024

Assigned to @geraldmaboshe! Please open a draft PR linking this issue within 48h ⚠️ If we can't detect a PR from you linking this issue in 48h, you'll be unassigned automatically 🕹️ Excited to have you ship this 🚀

@harshsbhat
Copy link
Contributor Author

@geraldmaboshe feel free to refer my closed PR

@geraldmaboshe
Copy link
Contributor

cool, thanks for the cheatsheet @harshsbhat 😃

Copy link

oss-gg bot commented Oct 6, 2024

@NeerajReddyK, Just a little reminder: Please open a draft PR linking this issue within 12 hours. If we can't detect a PR in 12h, you will be unassigned automatically.

@anand1564
Copy link

/assign

Copy link

oss-gg bot commented Oct 7, 2024

This issue is already assigned to another person. Please find more issues here.

@harshsbhat
Copy link
Contributor Author

@anand1564 there is already a PR open for this issue

Copy link

oss-gg bot commented Oct 8, 2024

@NeerajReddyK, Just a little reminder: Please open a draft PR linking this issue within 12 hours. If we can't detect a PR in 12h, you will be unassigned automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants