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

fix dead lock #2716

Merged
merged 6 commits into from
Jul 29, 2024
Merged

fix dead lock #2716

merged 6 commits into from
Jul 29, 2024

Conversation

YarBor
Copy link
Contributor

@YarBor YarBor commented Jul 25, 2024

No description provided.

YarBor added 2 commits July 25, 2024 12:49
Signed-off-by: YarBor <yarbor.ww@gmail.com>
Signed-off-by: YarBor <yarbor.ww@gmail.com>
Copy link
Contributor

@chickenlj chickenlj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Contributor

@FoghostCn FoghostCn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@FoghostCn FoghostCn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls add license for router_test.go

@YarBor YarBor requested a review from FoghostCn July 26, 2024 03:16
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plz write unit test like other routers

Comment on lines 157 to 159
want: func(invokers []protocol.Invoker) bool {
return true
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

Comment on lines 129 to 131
want: func(invokers []protocol.Invoker) bool {
return true
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the point

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expect to do nothing,the script will make panic, and recover, then ignore script route

Signed-off-by: YarBor <yarbor.ww@gmail.com>
Copy link

sonarcloud bot commented Jul 26, 2024

@YarBor YarBor requested a review from FinalT July 26, 2024 04:04
@AlexStocks
Copy link
Contributor

先不要着急合并,看下 ci 为啥没过

@chickenlj
Copy link
Contributor

先不要着急合并,看下 ci 为啥没过

There were environmental issues on the CI platform that caused all PRs to fail. We need to find time to take a closer look and fix them before the official version. The current user is waiting to verify the new version, so we need to merge it first

@chickenlj chickenlj merged commit 8747aa9 into apache:main Jul 29, 2024
4 of 5 checks passed
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.

5 participants