Time Nick Message 09:29 Zughy[m] reminder that there are 3 ">= 2 approvals" PRs ready to be merged and 16 PRs with one approval (3 of them by core devs, so they just need a self-approval) 11:17 Anim_artel Hello, 11:18 Anim_artel Sorry if I'm not in the right place, I'm looking for the people who take care of the wiki... 11:18 Anim_artel I'm testing on discord too :D 11:22 Anim_artel Mostly because there, I could read messages posted after I quit :)) 11:24 pgimeno you can also read them here: https://irc.minetest.net/minetest-dev/today 11:26 Anim_artel cool, thanks ! 11:27 Anim_artel I hope I'll be able to chat with you, I'm better in French... 11:45 nrz merging #13524 11:45 ShadowBot https://github.com/minetest/minetest/issues/13524 -- Reset day/night ratio even when passing no arguments by Zughy 11:47 nrz merging #13525 11:47 ShadowBot https://github.com/minetest/minetest/issues/13525 -- Reset player lighting when passing no arguments by Zughy 11:48 nrz and #13514 (for now) 11:48 ShadowBot https://github.com/minetest/minetest/issues/13514 -- Disable `desynchronize_mapblock_texture_animation` by default by rollerozxa 11:53 Anim_artel I have to go, I found someone on discord, but I would also look at the logs from here 11:54 Anim_artel goodbye :) 12:44 Zughy[m] didn't we have a "beginner friendly" label? There isn't anymore 12:45 ROllerozxa "good first issue"? 12:52 rubenwardy yeah, changed as GitHub recognises "good first issue" and not "beginner friendly" 12:53 rubenwardy should probably update links 12:55 Zughy[m] oh lol, I thought that "good first issue" meant "that's a very useful report, newcomer" 13:13 nrz merging #13506 13:13 ShadowBot https://github.com/minetest/minetest/issues/13506 -- Convert spaces to tabs by Bituvo 13:21 nrz and merging #13528 13:21 ShadowBot https://github.com/minetest/minetest/issues/13528 -- Android build via CMake by sfan5 13:24 sfan5 oh huh 13:24 sfan5 that was quick 13:31 Zughy[m] nrz going yolo 13:57 nrz no, i just took time for the project to re-review MR and merge those in the conditions to be mergeable 13:57 nrz yeah sometimes i appear and review 😄 13:58 nrz i have some ideas for the project but i miss time, then as it's big interesting refacto i won't, but i follow this chan closely, a bit less GH 😄 14:06 rubenwardy technically self-approvals aren't automatic 14:06 rubenwardy very good change though 14:11 ROllerozxa hmm, if the android cmake PR has been merged, would #13215 be next? ;) 14:11 ShadowBot https://github.com/minetest/minetest/issues/13215 -- Add IrrlichtMT as a submodule by numberZero 14:34 imi hi, currently the screwdriver isn't repairable with the anvil, and I'm trying to make that happen. my current approach is to make an on_rotate callback for the anvil, prevent rotation and transfer the screwdriver to the anvil. as far as I can tell set_wielded_item doesn't work from an on_rotate function 14:37 imi despite returning true 14:42 nrz Rollerozxa: i'm against this PR, as i said submodule is always interesting at first, then it's a pure hell to maintain. I think in OSS it's a suicide, as most people will forget git submodule update --init with each git pull and it will leads to more support 14:43 nrz for me irrlichtMT won't be redistributed and is not used by any other third party, just do a mono repo 14:43 nrz at least we ship a fully working together code and less headhacked and PR to say, submodule update too 14:50 nrz ah i didn't put the comments in the issue at first, it's put, and i illustrated what i said with articles explaining better than me 14:51 nrz but i had to work with submodules multiple times in the past 10 days, and it was always a pain at a moment for teams, and majority finished with removal of the submodule and reintegrated in caller repo, as most of those submodule was not for redistributing to other teams 14:52 rubenwardy if we decide not to go with a submodule, how much more needs to be done before irrlicht can just be added to the main repo? 14:57 nrz not so much as it's already downloaded and included as a cmake module no ? sfan5 ? 14:58 nrz if i remember, we put currently irrlicht in lib/irrlicht using a tarbal, and then build no ? then if it's already in tree ? 🙂 14:59 Desour the current workflow is to clone irrlicht into lib/irrlichtmt, you don't have to use tar balls 15:01 nrz then it's exactly same if you already have sources 😄 15:01 nrz less the submodule hell 15:03 imi +1 for submodules on my side 15:03 nrz who always work with submodule on a real project, i think only people who worked with it can vote :p 15:04 MTDiscord +1 for submodules from my side as someone who has worked with them, and frankly the current build process is essentially submodules at home already (i.e. worse) 15:10 MTDiscord I have worked with submodules to maintain aes_game, when I was running my own server. Personally I liked it as someone who was getting into more advanced git. Even I often forgot to pull/update submodules, though, and it caused me a few headaches. Someone not used to git would have trouble. Keep in mind, minetest is often peoples' first intro to git 15:11 MTDiscord So, -1 from me, lets leave good enough alone until we integrate it into the main repo anyhow 15:12 MTDiscord Submodules are absolutely terrible ... almost as bad as every other option. 15:13 MTDiscord The best option is to have a monolithic repo, which is the plan anyhow. Keeping that in mind, whatever is decided doesn't matter in the long run 16:39 Zughy[m] Hello, I think we've got a situation here: https://github.com/minetest/minetest/pull/10100#issuecomment-1564613670 16:39 Zughy[m] I think nephele is right, but I also think the issue lies elsewhere: we're not authorative enough when it comes down to closing PRs. We've got a roadmap and the initial rule was that core devs had 14 days to support a PR outside the roadmap (and supporting it meant reviewing it, at least for the core dev who opted for supporting it). Then people complained so we stopped, as if we didn't want to hurt them. But keeping such PRs stalling for 16:39 Zughy[m] months (https://github.com/minetest/minetest/pulls?q=is%3Apr+is%3Aopen+label%3A%22Roadmap%3A+Needs+approval%22) has the same effect, added to the psychological effect to see way more PRs in the total amount (in one word: pressure). I propose to go back to the original 14 day rule, because it seems crystal clear that we can't deal with all this work nor I want to see some core devs doing heroics to then burn out. I can work out a message 16:39 Zughy[m] to post when closing such PRs (as a triager, it's my role in the end). Thoughts? 17:25 MTDiscord out of curiosity, why did it stall for months. it looked like it was ignored from sept 21 to june 22. i'm not sure why it was engaged with just to ignore. if a review process starts, it should probably continue as the highest priority if it's the oldest 17:26 MTDiscord it's like getting hopes up, just to knock them down 17:26 MTDiscord closing down pr's because they're old hides the symtoms of the issue and not the issue 17:27 MTDiscord (if the author ignores it and doesn't engage, it's probably on them to open it back up again) 17:31 MTDiscord reviewing code sucks, and maybe if you've created 3/4 prs and haven't reviewed anything, pick up 1 review before starting next thing if there is something older than 1 month. unless it's a dreadful pr. not sure what the policy is for that, but maybe just being honest with someone. not keen on this for x, y, and z reasons. sorry 17:32 MTDiscord there is a bit of the, i could probably do more if i write code, rather than review, but if you review, and build people up, you bring people in, and in 6 months time, your workload is less, but if you don't review, it'll always only ever be on a select group 17:33 MTDiscord pr's aren't an acceptance or rejection only, it's a conversation, a learning exercise, a training exercise and a way to discuss things that haven't really had a decision on. it's more about the people and conversations than the code imho 17:40 MTDiscord perhaps those that review the most pr's get a writeup in the blog that month from other core devs saying how fantastic they are. i'm sure that'd motivate other core devs to review more. cannot get ego's getting out of check 🙂 20:34 Zughy[m] AncientMariner: the issue is that we haven't got enough manpower to cover all these PRs, and people might find themselves in the situation where they have to wait for months, if not years. The roadmap is a way to focus only on certain areas of development, but if it's not enforced, it's pretty useless 20:35 Zughy[m] (of couse any core dev can say "no ok, I think this PR is important anyway, I'll take care of it") 20:35 Zughy[m] *course 20:36 MTDiscord there are so many awesome prs that get ignored 20:36 MTDiscord reviewing pr's building the confidence and skills of contributors is the best way to have more manpower. otherwise you're waiting for someone to come along with the perfect skills, knowledge, and and unflappable attitude that can wait months/years for a review and by that time they are motivate to join in and solve those problems. that person is also known a silver bullet or unicorn, and there are none 🙂 20:37 MTDiscord i'll be as delicate as i can with the concept of the roadmap... (it's bullshit :)). if you want a roadmap and want all core devs to focus on key areas, great, good plan, good strategy, but if you are hoping people come along with perfect skills and only focus on the areas you're mandating, it just doesn't happen much... 20:38 Zughy[m] reviewing PRs to build confidence requires to review PRs in the first place 20:40 MTDiscord which is an active decision, and a choice. you're actively choosing to close and hide them so they don't need to be reviewed rather than thinking how can we get a little better at reviewing. (do you break it down, encourage, prioritise it). prioritizing new contributors isn't a bad investment, and the projects that do that will have achieved much more after 2 years 20:41 MTDiscord the roadmap is a tool for focus, it's an aid to achieve, it's not the boss of you. it hasn't become this all knowing entity that you submit your soles to and step out of the driving seat 20:42 Zughy[m] we've got at least one dev who gets burnouts once in a while, so to avoid that they opt for denial. Improving the reviewing process is not feasible, also because it's a project made of volunteers 20:43 Zughy[m] I'm not saying MT should commit to the roadmap and the roadmap only. I'm saying that closing PRs outside the roadmap that don't get anyone's attention after 14 days is a way of avoid the white noise around 20:43 Zughy[m] *to avoid 20:44 MTDiscord any person that gets burnout should be protected, and that is a team effort. if you care about them, step in and share the workload. i know it's a volunteer project (i'm a maintainer of a project with 785 issues) 20:45 MTDiscord but it's not about avoiding old issues, if you have 2 hours in the week, spend 1.5 working and 0.5 reviewing, rather than 2 working and chip away at it, it's about choosing what to focus on, and reviewing isn't sexy, but it is important 20:45 rubenwardy the [Supported by core dev] system is how you get concept approvals for things not on the roadmap 20:46 Zughy[m] I do, AncientMariner (protecting people from burning out), yet unfortunately sometimes the burnout becomes real for me as well 20:46 Zughy[m] too much burden that turns into denial imho 20:47 MTDiscord i get it, and it's a system that has been chosen around the almighty roadmap. prioritise roadmap activities and chip away, even if it's 1 pr per month, and reviewing someone elses, rather than not reviewing at all. i'm not saying do more, i'm saying prioritizing new contributors is a worthwhile investment 20:48 Zughy[m] in case it's not clear: I'm not a core dev, my C++ knowledge is shit, I'm just one the people tidying up the place 20:48 MTDiscord and i get the concept about burnout, i've felt it at times, and it's about accepting what you can and cannot do, and having realistic expectations 20:49 MTDiscord it doesn't mean ignore other things though. it's a strategy, but won't necessarily help with the burnout 20:49 MTDiscord it will probably make it worse (because you'll feel who else will do this but me), and the answer is no one, because those other people got fed up waiting for review 20:49 MTDiscord it's about building an organisation that onboards people and shares the burden, a welcoming culture 20:50 MTDiscord many tech leads in work don't do work, their focus become on helping get the best out of others, and it isn't always sexy and fun 20:50 MTDiscord but it is important, and the skills you guys have in the team there is at least 4/5 tech leads in terms of ability 20:52 MTDiscord i appreciate it, c++ is hard, and it requires a really niche skill set, those c++ devs are talented folk. they deserve help and support 20:53 MTDiscord i'm suggesting one way to do it, and i understand i'm an outside ("hey, what does he know"), but i do see a way to improve it, and i'll say it, even if folk think i'm chatting shit 🙂 23:27 MTDiscord Does minetest.get_perlin_map even exist? It is documented in the api, but it returns nil when PerlinNoiseMap does not 23:28 MTDiscord And yes, ive been playing around with mglua, but no, this is in the regular mapgen environment 23:28 MTDiscord on that branch, though 23:32 MTDiscord Hello MisterE, this is the best I could do when searching for the integration of that classic function for you https://github.com/minetest/minetest/blob/master/src/script/lua_api/l_env.cpp#L1043 23:33 MTDiscord Which mapgen versions are you testing it on? Perhaps that could be helpful information 23:41 MTDiscord so... it exists. So, IDK