Skip to content

chore: Apply a proper logic for NotificationQueue.ts#33851

Merged
sampaiodiego merged 3 commits intoRocketChat:developfrom
Lao-Ax:fix-NotificationQueue
Dec 13, 2024
Merged

chore: Apply a proper logic for NotificationQueue.ts#33851
sampaiodiego merged 3 commits intoRocketChat:developfrom
Lao-Ax:fix-NotificationQueue

Conversation

@Lao-Ax
Copy link
Contributor

@Lao-Ax Lao-Ax commented Oct 31, 2024

It's a quite minor change. But the current variant crashes the terminal in my synthetic tests.

In the current code the line worker(counter++) passes the previous state of the counter, I mean before the increment. So, it is literally always the same as worker(0); counter = counter + 1.
But it looks like that the intention was: worker(counter + 1) to start a new worker with next number.

Issue(s)

CORE-851

@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Oct 31, 2024

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Oct 31, 2024

⚠️ No Changeset found

Latest commit: 6305ea2

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@CLAassistant
Copy link

CLAassistant commented Oct 31, 2024

CLA assistant check
All committers have signed the CLA.

@ggazzo ggazzo added this to the 7.1.0 milestone Nov 1, 2024
@casalsgh casalsgh modified the milestones: 7.1.0, 7.2.0 Nov 26, 2024
The line `worker(counter++)` passed the previous state of the counter, I mean befor increment. So it is the same like `worker(0); couner = counter + 1`
@Lao-Ax Lao-Ax force-pushed the fix-NotificationQueue branch from 3bbe686 to d75d42b Compare December 6, 2024 06:11
@Lao-Ax Lao-Ax changed the title Update NotificationQueue.ts fix: Apply a proper logic for NotificationQueue.ts Dec 6, 2024
@KevLehman KevLehman changed the title fix: Apply a proper logic for NotificationQueue.ts chore: Apply a proper logic for NotificationQueue.ts Dec 10, 2024
@sampaiodiego sampaiodiego merged commit 85980d7 into RocketChat:develop Dec 13, 2024
@sampaiodiego
Copy link
Member

thx @Lao-Ax , good catch ;)

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.

6 participants