You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by GitBox <gi...@apache.org> on 2022/09/25 03:12:44 UTC
[GitHub] [incubator-kvrocks] git-hulk opened a new pull request, #919: Fix many typo in cluster.cc
git-hulk opened a new pull request, #919:
URL: https://github.com/apache/incubator-kvrocks/pull/919
1. Fix typos in the cluster.cc
2. Use const string when appearing many times
3. Fix incorrect sleep time in rwlock test case
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on pull request #919: Fix many typos in cluster.cc
Posted by GitBox <gi...@apache.org>.
git-hulk commented on PR #919:
URL: https://github.com/apache/incubator-kvrocks/pull/919#issuecomment-1257490567
> > hi @PragmaTwice, seems the namespace of `chrono_literals` is a bit different between compilers, do you suggest rollbacking to use the previous way?
>
> Sure. Maybe it need c++14 🤣
I was a bit confuse when looking its docs, it says `std::literals::chrono_literals` required at least C++14, but std::chrono_literals can be found in C++11's <chrono>. Let's use the legacy way first.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on pull request #919: Fix many typos in cluster.cc
Posted by GitBox <gi...@apache.org>.
git-hulk commented on PR #919:
URL: https://github.com/apache/incubator-kvrocks/pull/919#issuecomment-1257454840
hi @PragmaTwice, seems the namespace of `chrono_literals` is a bit different between compilers, do you suggest rollbacking to use the previous way?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #919: Fix many typo in cluster.cc
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #919:
URL: https://github.com/apache/incubator-kvrocks/pull/919#discussion_r979423320
##########
tests/cppunit/rwlock_test.cc:
##########
@@ -167,7 +167,7 @@ TEST(ReadWriteLock, WriteLockGurad_First) {
});
} else {
ths[i] = std::thread([&rwlock, &val]() {
- usleep(1000); // To avoid it is the first to run, just sleep 100ms
+ usleep(100000); // To avoid it is the first to run, just sleep 100ms
Review Comment:
```suggestion
using namespace std::chrono_literals;
std::this_thread::sleep_for(100ms); // To avoid it is the first to run, just sleep 100ms
```
We can use the time literal `100ms` directly, haha : )
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on pull request #919: Fix many typos in cluster.cc
Posted by GitBox <gi...@apache.org>.
git-hulk commented on PR #919:
URL: https://github.com/apache/incubator-kvrocks/pull/919#issuecomment-1257438708
> I notice that CI is down in darwin clang. Maybe `#include <chrono>` is needed to use namespace `std::chrono_literals`.
Yes, should include `<chrono>`, fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #919: Fix many typos in cluster.cc
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on PR #919:
URL: https://github.com/apache/incubator-kvrocks/pull/919#issuecomment-1257417819
I notice that CI is down in darwin clang. Maybe `#include <chrono>` is needed to use namespace `std::chrono_literals`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #919: Fix many typo in cluster.cc
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #919:
URL: https://github.com/apache/incubator-kvrocks/pull/919#discussion_r979423320
##########
tests/cppunit/rwlock_test.cc:
##########
@@ -167,7 +167,7 @@ TEST(ReadWriteLock, WriteLockGurad_First) {
});
} else {
ths[i] = std::thread([&rwlock, &val]() {
- usleep(1000); // To avoid it is the first to run, just sleep 100ms
+ usleep(100000); // To avoid it is the first to run, just sleep 100ms
Review Comment:
```suggestion
using namespace std::chrono_literals;
std::this_thread::sleep_for(100ms); // To avoid it is the first to run, just sleep 100ms
```
We can use the time literal `100ms` (defined in chrono part of std library) directly, haha : )
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #919: Fix many typos in cluster.cc
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on PR #919:
URL: https://github.com/apache/incubator-kvrocks/pull/919#issuecomment-1257458703
> hi @PragmaTwice, seems the namespace of `chrono_literals` is a bit different between compilers, do you suggest rollbacking to use the previous way?
Sure. Maybe it need c++14 :rofl:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on pull request #919: Fix many typos in cluster.cc
Posted by GitBox <gi...@apache.org>.
git-hulk commented on PR #919:
URL: https://github.com/apache/incubator-kvrocks/pull/919#issuecomment-1257739226
Thanks all, merging...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk merged pull request #919: Fix many typos in cluster.cc
Posted by GitBox <gi...@apache.org>.
git-hulk merged PR #919:
URL: https://github.com/apache/incubator-kvrocks/pull/919
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org