Time  Nick       Message
17:03 Krock      nerzhul, nore, paramat, rubenwardy, sfan5, Shara, sofar - I would like to do a meeting in ~an hour (or later, if desired) to discuss the points here: https://github.com/orgs/minetest/teams/engine/discussions/9
17:04 paramat    i'll be here
17:04 sfan5      szre
17:04 sfan5      sure*
17:04 Krock      Please add there more points if you'd like to discuss them in the meeting
17:04 Krock      Great :)
17:20 Shara      Should be here
17:38 nerzhul    i will lunch but go for 0.4.17
17:42 * VanessaE growls at paramat
17:47 paramat    sorry for that :3
18:03 Krock      nore, paramat, sfan5, Shara, sofar. I'd like to start with our meeting now. We're about to discuss about 0.4.17 again
18:03 Krock      also rubenwardy ^
18:03 VanessaE   sounds like I piped up just in time :P
18:04 Krock      Question is when. When do we want to release 0.4.17? In the last weeks I've visited a view servers to see how it's going and ran singleplayer worlds
18:04 Krock      Unlike 0.5.0 it doesn't suffer from occasional segfaults and is surely stable enough for a release
18:05 sfan5      The question is how many more fixes / adjustments from 0.5 we want backported into 0.4.17
18:06 VanessaE   well, fwiw my servers have been running 3d0617c9 for some time, no engine faults that I've seen, that aren't already known.
18:06 Krock      at one point we've got to say it's enough for a release - because #7297 would be a backport candidate too
18:06 ShadowBot  https://github.com/minetest/minetest/issues/7297 -- Fix builtin inventory list crash when size = 0 by SmallJoker
18:06 VanessaE   sfan5: maybe just satisfy the current milestones/backports list?
18:08 Krock      since we're doing a 0.4.18 release anyway I don't see a reason to delay this release for much longer - in a month it's a year since 0.4.16
18:09 sfan5      VanessaE: I don't think there's much queued up for backporting
18:09 sfan5      no idea about te milestone list
18:09 sfan5      Krock: sounds good
18:09 VanessaE   sfan5: didn't think so.
18:09 VanessaE   (been a while since I looked at the list)
18:10 Krock      Only one of paramat's PRs is still in the "official" queue to backport if I see that correctly (#6746)
18:10 ShadowBot  https://github.com/minetest/minetest/issues/6746 -- Backport 0.4 by sfan5
18:14 Krock      Okay. Hopefully more devs will be active in the later state of this meeting.
18:16 Krock      paramat, you've added the discussion point about the issue number or PR number in the merge commits
18:16 Krock      what's the particular issue with it? IMO it's helpful to see where the commit comes from - also to get to the discussion easily
18:20 p_gimeno   I didn't want to interfere with the meeting, but since there seems to be little activity, I wanted to note that it's possible (and perhaps desirable) to make #6898 backportable
18:20 ShadowBot  https://github.com/minetest/minetest/issues/6898 -- Allow distinguishing mods by modpack by pgimeno
18:21 Krock      p_gimeno, for that it's an announced meeting it's quite dead, sadly. I'm not quite sure whether that may be backported. It's not backwards compatible once the map config is saved
18:22 Krock      but it's surely an important bugfix
18:23 p_gimeno   Krock: it can be made backwards compatible, by using numbers for the "master" directories (mods, game...) and then make 0.5 accept numbers but save names
18:23 sfan5      that sounds "too major" to be backported
18:25 Krock      so hash the directory name and save it as number? well that idea isn't too bad but makes the file very hard to edit manually
18:25 p_gimeno   no no, just use 1.modname for the game mods (or 1.modpack.modname), 2.modname for the world mods, and 3.modname for the mods in the mods folder
18:27 p_gimeno   (as opposed to game.modname, world.modname, mods.modname as it is now)
18:27 Krock      atoi("1.modname") translates to a non-zero value? I'm impressed
18:27 p_gimeno   yeah
18:29 Krock      it's still quite hacky so I'd prefer to keep it non-backwards-compatible in 0.5.0-dev only. Another dev might have a different opinion on this
18:29 Krock      first of all it needs a rebase and the a 2nd approval
18:30 paramat    oops sorry, back now
18:30 Krock      paramat, take your time. I'm glad you appeared :)
18:31 paramat    there are a few commits that need backporting in MTG
18:32 paramat    the cavegen fix is very important, i'd like it to be in 0.4.17
18:34 Krock      paramat, what other commits need backporting? If they're not too important we can still release them in 0.4.18
18:36 paramat    the cavegen fix is a todo in engine backporting
18:37 Krock      actually I meant the "a few commits that need backporting". So it's only the one you've posted there?
18:38 paramat    in MTG, the todos in backports are not recent, and the last backport in MTG was a long time ago, so i feel they should be backported for 0.4.17. however i can't do this, not good enough with git yet
18:38 Krock      Ah, MTG. I see
18:39 paramat    sorry i'm not clear. the "few commits that need backporting" are MTG
18:40 paramat    https://github.com/minetest/minetest_game/pull/1966
18:40 paramat    "last updated: 2017-12-05"
18:41 paramat    so more really should be backported for 0.4.17
18:42 paramat    erm, i don't feel strongly about issue number link in commits, but it seems wrong for a commit message because it is a github-specific link. commit messages should not be github-specific
18:43 paramat    although i realise the links are useful
18:43 paramat    they also make commit messages look messy
18:44 paramat    none of our old commit messages had links in them, before this new way of merging PRs
18:44 paramat    anyway maybe i'll need to open an issue for more to see
18:45 Krock      It's not a must-do but these eight additional characters " (#nnnn)" don't hurt
18:47 Shara      I kind of share paramat's view on this for whatever it's worth
18:48 Krock      if the links are useful to you too, then we should keep it. Short titles are advertised like everywhere too
18:49 Krock      Shara, in your opinion they clutter the titles?
18:49 paramat    i strip off the link in MTG, but maybe we can come to a decision sometime for the engine
18:49 Shara      Yea. I find them just pure messy and feel a commit should stand up fine on its own without needing that link.
18:51 Krock      It's just when you've got a commit, say, https://github.com/minetest/minetest/commit/bcd22fc34 . The fastest way to get to the PR discussion is the link. One click away
18:51 paramat    yes
18:51 Shara      I do get that viewpoint, just... messy :P
18:51 Krock      of course it is, but there's no other way to link them without a mess
18:52 Krock      Unless we want to pack the entire link into the description
18:52 Krock      which will cause more inconsistency anyway
18:53 paramat    for me the arguments against are perhaps strong enough to override the advantage :]
18:53 Shara      How often is it needed to go back to the discussion? That's probably what decides it
18:54 paramat    just don't link to the discussion, the discussion is a github-specific thing. if MT goes to another git site the links will be useless mess
18:54 sfan5      I find it very useful when reviewing a change because I'm looking to solve a problem, often only reading the PR will reveal why something was done
18:54 paramat    it's always possible to navigate to the discussion from a commit with a little work
18:55 sfan5      s/reviewing/triyng to understand/
18:55 sfan5      trying* ....
18:55 Krock      Shara, for my taste it's way too many times when I look up a PR this way, especially in the online git blame page
18:55 Shara      That's a good enough reason then
18:56 paramat    but anyway, i think consistency with commit history is enough to avoid them
18:56 Krock      sfan5, exactly. Also the PR discussion shows some ideas/alternatives or concerns which might be helpful later on
18:56 sfan5      yeah
18:57 Krock      paramat, the commit history is already inconsistent with the prefixes, such as "Lua_api: foo" vs "Fix server.cpp issue"
18:58 Krock      it doesn't have to be perfect. it has to be understandable and helpful.
18:59 paramat    but, for the engine i'm happy to go with the decision. but in MTG the active devs are against :)
19:00 Krock      the PRs in MTG are quite overviewable, yes
19:00 paramat    so, in engine i'll leave them in from now on
19:01 Krock      leave them when they're inserted automatically and remove them in MTG if you want to do so
19:01 paramat    yeah
19:02 paramat    erm i guess 0.4.17 waits on rubenwardy 's spare time
19:02 Krock      after all we've got a ton more PRs on MTE so that deal seems fair
19:02 Krock      sfan5, any opinion on the MTG commit title format (automatic PR number insertion)?
19:03 sfan5      prefer with PR number
19:03 sfan5      but I wouldn't mind not having it
19:03 Krock      paramat, everyone who has push access to a repository can push to its PR branches
19:03 paramat    yes i know :)
19:04 rubenwardy If you merge with github, it'll give you an actual link in the commit to the pr
19:04 rubenwardy Iirc
19:05 Shara      Yes
19:05 Shara      I usually merge with github, so rely on that
19:05 Shara      example: https://github.com/minetest/minetest_game/commit/b52ea3de159a135bad608360ce8d68c57bf64ce7
19:05 paramat    but yes i agree to freezing 0.4.17 soonish, like 1-2 weeks? depends what is good for core devs
19:05 Shara      Link to commit isright at the top
19:06 Shara      is right*
19:06 paramat    but only announce on freeze, never before hehe
19:07 paramat    typical MT, it can't be rushed or planned
19:07 Krock      Shara, if you meant the PR/issue number: yes it's there. But look at these https://github.com/minetest/minetest/commit/574dab5c https://github.com/minetest/minetest/commit/43f98eb47
19:07 Krock      ^ GitHub doesn't always link them properly
19:08 Krock      ignore the first link, it was pushed directly
19:09 Shara      Hmm, haven't encountered it messing up before :)
19:10 Krock      paramat, we don't need more than a week. after the updated backports it's ready to be shipped
19:10 Krock      .. after a week
19:18 paramat    ok
19:19 paramat    so please can someone backport the MTG todo commits sometime?
19:26 nerzhul    why does 0.4.17 waits for rubenwardy ?
19:27 rubenwardy no idea
19:27 rubenwardy the only thing I'd like to do RE: 0.4.17 is run it on my server to find bugs
19:27 rubenwardy which I'm already doing
19:30 paramat    oh i wrote that because ruben wrote 'this week is not good for me anyway' so it sounded like he wanted to have spare time for 0.4.17
19:47 Krock      After trying to update the MTG backport branch  using  sed -r 's|[0-9]+ ([0-9a-f]+) .*|\1|gp' ../commits.txt | while read h; do git cherry-pick $h; if [[ $? -ne 0 ]]; then read -p "Done? "; fi done  I noticed I would've been faster by cherry-picking the new ones manually
19:48 Krock      forgot to mention that I failed horribly anyway..
19:49 sfan5      yeah just do it manually, it's much better
19:50 Krock      is that sarcasm or serious? I'm asking because a command like this would've saved some time .. if it worked properly
19:52 p_gimeno   Krock: xargs FTW :) (with -L 1 here maybe)
19:56 p_gimeno   <Krock> first of all it needs a rebase and the a 2nd approval <-- When should I rebase? I asked this some time ago but got no response: Apr 03 19:59:20 <p_gimeno>      on a different matter, I have a question. Should I rebase my PRs when I notice there's a conflict, or wait until being told to do so?
19:57 sfan5      Krock: serious
20:06 Krock      p_gimeno, sadly I can't control the PR review speed either. However, sometimes testing code and results help to speed it up a little
20:07 Krock      So when will we release 0.4.17? Next week? In two weeks?
20:09 Krock      rubenwardy, to be fair your server is also a bit salted and doesn't run MTG - but it checks the engine quite good
20:09 rubenwardy true
20:09 rubenwardy salted?
20:10 rdococ     Ready-salted
20:11 Krock      rubenwardy, salted/dirty - including patches that aren't part of 0.4.x
20:12 rubenwardy ah right
20:14 Krock      nerzhul, if you're still active: I've updated #7297 to address your comment. Is it good now?
20:14 ShadowBot  https://github.com/minetest/minetest/issues/7297 -- Fix builtin inventory list crash when size = 0 by SmallJoker
20:16 p_gimeno   Krock: um, the question was about when to rebase in case of conflicts, not about PR review speed
20:16 p_gimeno   should I go ahead and rebase it?
20:17 Krock      p_gimeno, oh yes, sure. Rebasing makes testing easier
20:18 p_gimeno   Krock: that's for sure, but obviously I don't want to be rebasing at every conflict while it isn't tested, hence my question of whether I should wait until told so. For example, if someone is willing to take a look at it, they can tell me to rebase. But rebasing just for the sake of it waiting for it to be looked into some day... is not fun.
20:20 Krock      p_gimeno, I know that it's tiring to rebase all the time, which happens to me sometimes aswell. However, inactive PRs are sometimes marked as "Neglect closure" - and can be easily revived by rebasing. Maybe after a while there are people willing to review
20:21 p_gimeno   ok, so I'm supposed to rebase it without being asked or anything, right?
20:21 rubenwardy the modpack PR is on my todo list to review, it's just exam season for me :(
20:30 Krock      Merging #7297 in 5 minutes. Assuming nerzhul's change request is done as desired
20:30 ShadowBot  https://github.com/minetest/minetest/issues/7297 -- Fix builtin inventory list crash when size = 0 by SmallJoker
20:36 Krock      merging...
20:42 nerzhul    Krock yeha
21:03 paramat    ahh thanks for doing MTG backport
21:04 paramat    i'll update the thread
21:11 paramat    perhaps we can freeze tomorrow
21:12 Krock      Honestly I hope so