Support new lane:join() API#137
Conversation
|
It sounds like testing this in CI might prove to be more brittle than just doing it. I'm inclined not to. |
|
Is this a place where checking the version string might be better? |
Aire-One
force-pushed
the
fix/#135
branch
3 times, most recently
from
0c417df to
aeb0e89
Compare
|
@daurnimator thank you for the suggestion. Rebased to use the version string exposed by lanes instead of the number of parameters. |
| end | ||
|
|
||
| local lanes_major = tonumber(lanes.ABOUT.version:match("^(%d+)")) | ||
|
|
There was a problem hiding this comment.
Instead of having the if in the loop below, have a function that adds backwards compat up here.
| -- Manage both new and old lane:join() API formats. | |
| -- See https://github.com/LuaLanes/lanes/commit/bfdc7a92c4e3e99522abb6d90ef2cbb021f36fc8 | |
| local worker_join_compat | |
| if lanes_major >= 4 then | |
| -- New API: {true, _, ok, worker_results} | |
| worker_join_compat = function(worker) return worker:join() end | |
| else | |
| -- Old API: {true, ok, worker_results} | |
| worker_join_compat = function(worker) | |
| local err, ok, worker_results = worker:join() | |
| return true, err, ok, worker_results | |
| end | |
| end |
There was a problem hiding this comment.
FYI I'm not sure if my implementation of the old API is actually correct. I didn't look very hard at what the new return values mean
Aire-One
force-pushed
the
fix/#135
branch
2 times, most recently
from
bd38da0 to
d1d80dc
Compare
|
|
||
| for _, worker in ipairs(workers) do | ||
| local _, ok, worker_results = assert(worker:join()) | ||
| local _, _, ok, worker_results = worker_join_compat(worker) |
| end | ||
|
|
||
| local lanes_major = tonumber(lanes.ABOUT.version:match("^(%d+)")) | ||
|
|
There was a problem hiding this comment.
FYI I'm not sure if my implementation of the old API is actually correct. I didn't look very hard at what the new return values mean
3 participants
Fix: #135
@alerque I went with a check on the number of retuned values to differentiate the old and new APIs. Does it work for you? Also, do you think we need some kind of tests (by mock or installing both versions in the CI) to ensure the code works with different versions of lualanes?