| Time |
Nick |
Message |
| 00:01 |
|
argyle77 joined #minetest-dev |
| 00:51 |
|
argyle77 joined #minetest-dev |
| 01:57 |
|
epod joined #minetest-dev |
| 02:44 |
|
galaxie joined #minetest-dev |
| 02:44 |
|
galaxie left #minetest-dev |
| 02:51 |
|
paramat joined #minetest-dev |
| 07:16 |
|
kaeza joined #minetest-dev |
| 07:41 |
|
ssieb joined #minetest-dev |
| 07:45 |
|
proller joined #minetest-dev |
| 09:15 |
|
YuGiOhJCJ joined #minetest-dev |
| 09:17 |
|
Beton joined #minetest-dev |
| 09:47 |
|
Darcidride joined #minetest-dev |
| 09:52 |
|
Lunatrius joined #minetest-dev |
| 10:51 |
|
proller joined #minetest-dev |
| 11:20 |
nerzhul |
merging #8218 |
| 11:20 |
ShadowBot |
https://github.com/minetest/minetest/issues/8218 -- Don't regain breath while in ignore node by Wuzzy2 |
| 11:23 |
|
Fixer joined #minetest-dev |
| 11:28 |
nerzhul |
pushing a lint fix too |
| 13:54 |
|
argyle77 joined #minetest-dev |
| 14:07 |
|
kaeza joined #minetest-dev |
| 14:30 |
|
proller joined #minetest-dev |
| 14:35 |
|
twoelk joined #minetest-dev |
| 14:39 |
|
kaeza joined #minetest-dev |
| 16:15 |
|
proller joined #minetest-dev |
| 16:32 |
|
kaeza joined #minetest-dev |
| 16:46 |
|
proller joined #minetest-dev |
| 17:29 |
|
proller joined #minetest-dev |
| 17:33 |
|
Wuzzy joined #minetest-dev |
| 17:33 |
Wuzzy |
Sorry. :-( |
| 17:34 |
Wuzzy |
I thought paramat etc. would prevent it from being merged... |
| 17:34 |
Wuzzy |
Now. How do clean up the mess I caused? |
| 18:18 |
|
YuGiOhJCJ joined #minetest-dev |
| 19:00 |
|
kaeza joined #minetest-dev |
| 19:38 |
|
pgimeno_ joined #minetest-dev |
| 19:39 |
sfan5 |
merging #8181 in 5 minutes |
| 19:39 |
ShadowBot |
https://github.com/minetest/minetest/issues/8181 -- Fix coloured fog in main menu by random-geek |
| 19:51 |
Wuzzy |
Is there a rule that says that code submissions to the Lua API will only be accepted when they are also documented? |
| 19:51 |
Wuzzy |
because if not, that might explain why the documentation always has holes |
| 20:04 |
rubenwardy |
That's an implicit rule |
| 20:04 |
rubenwardy |
There's also no explicit rule on whether a PR should actually work |
| 20:04 |
rubenwardy |
Or an explicit rule that code should be LGpl licensed |
| 20:07 |
nerzhul |
Wuzzy it's implicit i think but may be added to the dev wiki i think |
| 20:07 |
nerzhul |
for the documentation i think it should be clear :s |
| 20:08 |
Wuzzy |
so... in other words, you do not accept PRs with missing documentation? |
| 20:08 |
Wuzzy |
if so, that's good |
| 20:09 |
Wuzzy |
because it seems that some undocumented functions managed to sneak in regardless... but luckily, that seems to be the exception |
| 20:24 |
|
paramat joined #minetest-dev |
| 20:26 |
paramat |
Wuzzy no problem, i didn't mention the lint failure because it was unrelated to your PR :) |
| 20:43 |
|
proller joined #minetest-dev |
| 20:52 |
|
proller joined #minetest-dev |
| 21:26 |
|
proller joined #minetest-dev |
| 21:34 |
|
fwhcat joined #minetest-dev |
| 21:55 |
|
Ruslan1 joined #minetest-dev |
| 22:00 |
paramat |
the RTT PR needs attention if anyone can |
| 22:01 |
* cheapie |
sits down and pets the PR or something |
| 22:07 |
Wuzzy |
paramat: make it blocker then? :D |
| 22:07 |
Wuzzy |
oh. |
| 22:08 |
cheapie |
It seems like there's a lot of back and forth about some random minor stuff that needs changed. Why can't a core dev just change that so we don't have to wait around so much? |
| 22:08 |
VanessaE |
paramat: merge it as-is, then immediately open up an issue for that bit of code that isn't needed. |
| 22:08 |
VanessaE |
because it WORKS NOW. |
| 22:09 |
VanessaE |
cheapie's right. |
| 22:10 |
cheapie |
I mean, I'm not sure how the MT dev policies are, but if someone submitted a PR for my software that I liked except for one minor thing, I'd just merge it and then change that. |
| 22:10 |
|
jcalve joined #minetest-dev |
| 22:11 |
paramat |
i need to look at the PR and see what the hold up is. also, 'works' is not good enough, code has to be good |
| 22:12 |
cheapie |
I think GitHub even lets you suggest changes in a review these days too. Would be a lot more useful than "The second statement does not do anything due to an unused local variable" |
| 22:13 |
VanessaE |
... |
| 22:13 |
cheapie |
I mean, I don't have much Cwhatever experience, but I have a very hard time figuring out what SmallJoker is saying should be changed - and to what. |
| 22:13 |
rubenwardy |
it should just be a straight revert |
| 22:14 |
rubenwardy |
the only comments should be if there is a rebase mistake |
| 22:14 |
rubenwardy |
not sure how experienced ANAND is with git, so that may be likely |
| 22:15 |
paramat |
will look at it. smalljoker has reviewed in depth so in the current absence of any other in-depth reviews it's a good idea to wait for their +1, they'll be active tomorrow |
| 22:15 |
sfan5 |
a straight revert might not be possible because some time has passed between the rtt commit being merged |
| 22:15 |
cheapie |
This might even be one of those things where some code wasn't quite up to standards, and then now that someone is doing something nearby it's suddenly a problem. |
| 22:16 |
cheapie |
This happened last time I submitted a PR to technic - I touched one thing in one file and now suddenly all of the code style issues in that file were somehow my problem :P |
| 22:16 |
cheapie |
(Yes, I know technic isn't the engine) |
| 22:17 |
sfan5 |
paramat: the comment by pgimeno definitely needs addressing |
| 22:17 |
sfan5 |
the line he commented on is a no-op, I doubt it was this way before the rtt "fix" |
| 22:18 |
paramat |
ok will look at that |
| 22:18 |
cheapie |
Those lines seem to have been unchanged: https://github.com/minetest/minetest/blob/07b1743d3db086f0f984968252d9e3ac71336a7e/src/network/connectionthreads.cpp#L1176 |
| 22:23 |
sfan5 |
paramat: there, i fixed the pr |
| 22:23 |
sfan5 |
you can merge it now |
| 22:25 |
paramat |
ok checking |
| 22:25 |
paramat |
thanks |
| 22:26 |
cheapie |
Testing it here, whenever it finishes downloading... |
| 22:26 |
paramat |
just found another mistake |
| 22:26 |
cheapie |
(yay for slow connections) |
| 22:26 |
paramat |
see my latest line comment |
| 22:28 |
paramat |
oh, nevermind |
| 22:30 |
cheapie |
OK, finally got a copy, now I can test it :P |
| 22:32 |
cheapie |
Seems good as far as I can tell *shrug* |
| 22:33 |
VanessaE |
applying to my servers and client, on top of clean HEAD (just now) |
| 22:39 |
paramat |
i'll try to review and merge it tonight |
| 22:40 |
VanessaE |
paramat: no offense, man, but I think you've done enough reviewing.... |
| 22:40 |
VanessaE |
:P |
| 22:40 |
VanessaE |
(of this PR anyway) |
| 22:43 |
paramat |
no, i haven't reviewed it at all, and should have done so much sooner |
| 22:44 |
|
jcalve left #minetest-dev |
| 22:44 |
VanessaE |
you know what I mean. |
| 22:57 |
paramat |
no i don't, what else can it mean? |
| 22:59 |
VanessaE |
all your commentary about what is or is not needed. |
| 22:59 |
paramat |
apart from spotting 2 mistakes i haven't looked at it at all |
| 22:59 |
VanessaE |
to me, that's as much of a "review" as going through the final PR. |
| 23:00 |
VanessaE |
anyway, that doesn't matter. |
| 23:00 |
sfan5 |
the question isn't really about the code since that's just a straight revert |
| 23:00 |
VanessaE |
sfan approved it, cheapie and I tested it. why does it need anything more? |
| 23:00 |
sfan5 |
the question is whether the revert should happen |
| 23:00 |
sfan5 |
which I'm pretty sure was already decided |
| 23:00 |
paramat |
that's not a complete review. now please don't encourage sloppiness and rule breakage, you don't like bugs after all |
| 23:01 |
cheapie |
Well, the old code worked and the new code doesn't, so I think it should either be fixed or reverted :P |
| 23:01 |
paramat |
PRs require 2 approvals, it's not really trivial as it isn't an auto-revert, code mistakes might have happened, and in fact did happen |
| 23:01 |
VanessaE |
(where "old" and "new" are relative to clean HEAD, without the PR :P ) |
| 23:02 |
paramat |
yes revert is best i think |
| 23:23 |
paramat |
+1 merging |
| 23:23 |
|
argyle77 joined #minetest-dev |
| 23:24 |
paramat |
(8187 that is) once checks complete |
| 23:24 |
paramat |
then RC2 tomorrow for, i suggest, a week of testing before release |
| 23:26 |
paramat |
only very minor blockers remain |
| 23:29 |
paramat |
LOL, now LINT is complaining about the formatting of 'clang-format on/off' lines we added to keep it happy. it just gets more ridiculous |
| 23:30 |
paramat |
anyway, merging |
| 23:40 |
VanessaE |
thanks, paramat. |
| 23:41 |
cheapie |
\o/ |
| 23:42 |
paramat |
woohoo |
| 23:43 |
* cheapie |
accidentally runs make -j176 and promptly runs out of RAM |
| 23:43 |
VanessaE |
servers updated. |
| 23:50 |
paramat |
thanks for testing |
| 23:50 |
paramat |
rubenwardy +1 for #8162 can i merge it? |
| 23:50 |
ShadowBot |
https://github.com/minetest/minetest/issues/8162 -- Update credits by rubenwardy |
| 23:50 |
rubenwardy |
well, it's missing translators |
| 23:51 |
rubenwardy |
It might be worth adding a new section for that |
| 23:54 |
paramat |
ok |
| 23:55 |
|
Taoki joined #minetest-dev |
| 23:55 |
paramat |
i might do some lua_api.txt improvements now as there are so many needed |
