-
Notifications
You must be signed in to change notification settings - Fork 26.5k
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 register config not take effect because of url simplified #4397
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, will merge when CI passes.
Pls. check my comment below, I realized your change causes CI failure.
...dubbo-registry-api/src/main/java/org/apache/dubbo/registry/integration/RegistryProtocol.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls. check my comment.
@imsunv Can you remove unused import sentence ? |
@cvictory ok |
f5fa418
to
9498ad4
Compare
Codecov Report
@@ Coverage Diff @@
## master #4397 +/- ##
============================================
+ Coverage 63.96% 63.97% +0.01%
Complexity 452 452
============================================
Files 769 769
Lines 33206 33206
Branches 5237 5237
============================================
+ Hits 21239 21244 +5
+ Misses 9546 9543 -3
+ Partials 2421 2419 -2
Continue to review full report at Codecov.
|
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package org.apache.dubbo.registry.integration; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
显示整个文件变化了,这个没法看啊。能处理下吗?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
调不回来了,我重新开个Pr 吧
5b17fc2
to
cbb30ac
Compare
@cvictory RegistryDirectory in master Line separator is CRLF, it will change to LF when committing |
cbb30ac
to
b905448
Compare
fixed it by keep Line separator with CRLF |
@beiwei30 already fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
fix #4341
when set dubbo.registry.simplified true. the dubbo.service.register won't take effect because of the register key was simplified and will use default value true;
boolean register = registeredProviderUrl.getParameter("register", true);
change providerUrl here