-
Notifications
You must be signed in to change notification settings - Fork 4
Feature/otp 27 upgrade #72
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
base: main
Are you sure you want to change the base?
Conversation
chore: remove unwanted changes
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.
why do we need this file?
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.
why do we need this file?
@@ -20,21 +20,10 @@ | |||
|
|||
-module(vmq_cluster_test_utils). | |||
|
|||
-export([ |
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.
can we restore the previous formatting to easily get the actual diff?
@@ -328,4 +329,4 @@ start_listener(Config) -> | |||
ssl_path(File) -> | |||
Path = filename:dirname( | |||
proplists:get_value(source, ?MODULE:module_info(compile))), | |||
filename:join([Path, "ssl", File]). | |||
filename:join([Path, "ssl", File]). |
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.
please add the EOF line.
@@ -39,4 +39,4 @@ simple_healthcheck_test(_) -> | |||
application:ensure_all_started(inets), | |||
{ok, {_Status, _Headers, Body}} = httpc:request("http://localhost:8888/health"), | |||
JsonResponse = jsx:decode(list_to_binary(Body), [return_maps, {labels, binary}]), | |||
<<"OK">> = maps:get(<<"status">>, JsonResponse). | |||
<<"OK">> = maps:get(<<"status">>, JsonResponse). |
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.
please add the EOF line.
@@ -235,4 +235,4 @@ histogram_systree_test(_Cfg, Suffix, Val) -> | |||
Publish = packet:gen_publish(PublishTopic, 0, integer_to_binary(Val), []), | |||
ok = packet:expect_packet(SubSocket, "publish", Publish), | |||
gen_tcp:send(SubSocket, packet:gen_disconnect()), | |||
gen_tcp:close(SubSocket). | |||
gen_tcp:close(SubSocket). |
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.
please add the EOF line.
@@ -136,4 +136,4 @@ seed_rand(Config) -> | |||
|
|||
rand_bytes(N) -> | |||
L = [ rand:uniform(256)-1 || _ <- lists:seq(1,N)], | |||
list_to_binary(L). | |||
list_to_binary(L). |
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.
please add the EOF line.
@@ -547,4 +548,4 @@ exp_nothing(Timeout) -> | |||
ok | |||
end. | |||
|
|||
|
|||
|
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.
please add the EOF line.
|
||
RUN apk --no-cache --update --available upgrade && \ | ||
apk add --no-cache ncurses-libs openssl libstdc++ jq curl bash snappy-dev && \ | ||
apk add --no-cache ncurses openssl libstdc++ jq curl bash snappy libgcc libc6-compat && \ |
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.
any reason for updating ncurses-libs and snappy-dev?
(echo -e "\nContents of retry.spec\n" && | ||
cat $FULLPATH && | ||
echo -e "\nRetry suites:" && | ||
echo $(pcregrep -o2 -o3 --om-separator="/" -M "^{(cases|groups),\"(.+)\",[^\w]*(\w+),(.|\n)*?\.$" $FULLPATH | uniq | paste -s -d, -) && | ||
make db-reset && | ||
./rebar3 ct --suite=$(pcregrep -o2 -o3 --om-separator="/" -M "^{(cases|groups),\"(.+)\",[^\w]*(\w+),(.|\n)*?\.$" $FULLPATH | uniq | paste -s -d, -)) | ||
./rebar3 ct --suite=$(pcregrep -o2 -o3 --om-separator="/" -M "^{(cases|groups),\"(.+)\",[^\w]*(\w+),(.|\n)*?\.$" $FULLPATH | uniq | paste -s -d, -)) |
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.
please add the EOF line.
Can you include vanilla Vernemq PR or reference alongside the commit link where you made the specified fix/change in this PR description? |
- run: make rel | ||
# - run: make db-setup | ||
- run: pwd | ||
- run: sh ./run-tests-with-retry.sh . |
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.
please add the EOF line.
@@ -38,16 +36,17 @@ | |||
]}. | |||
|
|||
{deps, [ | |||
{recon, "2.5.1"}, | |||
{lager, "3.8.0"}, |
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.
why are we removing lager?
@@ -121,7 +119,7 @@ | |||
{mkdir, "data/msgstore"}, | |||
{mkdir, "log/sasl"}, | |||
|
|||
{copy, "_build/default/bin/psql_migration", "bin/psql_migration"}, | |||
% {copy, "_build/default/bin/psql_migration", "bin/psql_migration"}, |
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.
If this is not required, then remove this from the comments as well.
{plumtree, {git, "https://github.com/vernemq/plumtree.git", {branch, "master"}}} | ||
{plumtree, | ||
{git, "https://github.com/vernemq/plumtree.git", | ||
{ref, "e91404882281ea66f40de8bac671a50f44da069b"}}} |
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.
why are we using a specific commit rather than master?
I can see that the commit id used is the same as the master.
@@ -762,7 +762,7 @@ | |||
[ | |||
{default, "3,4,131"}, | |||
{datatype, string}, | |||
{commented, "3,4"} | |||
{commented, "3,4,5"} |
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.
are we adding support for the MQTT 5 protocol?
@@ -637,7 +637,7 @@ | |||
%% - listener.wss.default = 127.0.0.1:880 | |||
{mapping, "listener.tcp.$name", "vmq_server.listeners", [ | |||
{default, { "{{mqtt_default_ip}}", {{mqtt_default_port}} }}, | |||
{datatype, ip}, | |||
{datatype, [ip]}, |
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.
Do we require this change for the OTP update?
@@ -32,7 +32,7 @@ | |||
expect_packet(Socket, Name, Expected) -> | |||
expect_packet(gen_tcp, Socket, Name, Expected). | |||
expect_packet(Transport, Socket, Name, Expected) -> | |||
expect_packet(Transport, Socket, Name, Expected, 5000). | |||
expect_packet(Transport, Socket, Name, Expected, 15000). |
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.
is this required?
Proposed Changes
Types of Changes
What types of changes does your code introduce to this project?
Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply. You can also fill these out after creatingthe PR. If you're unsure about any of them, don't hesitate to ask on the
mailing list. We're here to help! This is simply a reminder of what we are
going to look for before merging your code.
CODE_OF_CONDUCT.md
documentFurther Comments
If this is a relatively large or complex change, kick off the discussion by
explaining why you chose the solution you did and what alternatives you
considered, etc.