| Time |
Nick |
Message |
| 00:00 |
Wuzzy |
i am thinking about changes that would be technically functional but that simply dont align with minetest project goals |
| 00:00 |
MTDiscord |
<Jonathon> perhaps, but thats also going to get you into a discussion on what the minetest project goals are |
| 00:01 |
Wuzzy |
there *is* a rough idea, but that discussion is unfortunately not complete |
| 00:01 |
v-rob |
Well, we have a pull request for a medium-term roadmap, and merging that would be a logical first step |
| 00:02 |
Wuzzy |
Hmmm, there is a near-complete lack of any priotization for issues |
| 00:02 |
Wuzzy |
or PRs |
| 00:02 |
Wuzzy |
its all just a long list. i find it very hard to manage |
| 00:02 |
Wuzzy |
GitHub labels suck, tbh |
| 00:03 |
v-rob |
We've got labels like "High priority" and "Low priority", but they're usually used in subjective ways. |
| 00:03 |
Wuzzy |
yeah, i dont think they are useful at all |
| 00:03 |
v-rob |
SSCSM has a "High priority" label, but no one is touching it |
| 00:04 |
Wuzzy |
high priority is super rare and only used when its obvious anyway |
| 00:04 |
Wuzzy |
and also completely undefined, haha. |
| 00:04 |
|
lhofhansl joined #minetest-dev |
| 00:04 |
Wuzzy |
anyhow. any ideas for good reasons to close/reject a PR? |
| 00:05 |
Wuzzy |
the goal is to keep the PR count low and thus much more managable, because currently its way too high. juggling with >100 open PRs constantly is very hard |
| 00:05 |
rubenwardy |
roadmap, rejection, inactivity |
| 00:06 |
v-rob |
If the PR is inactive, close it because no one is going to work on it. |
| 00:06 |
Wuzzy |
"inactivty" is pretty much the only reason so far |
| 00:06 |
MTDiscord |
<Jonathon> should at least give warning/opportunity to others to continue it on |
| 00:07 |
v-rob |
Of course, things always go to "Possible close" before actually closing it |
| 00:07 |
Wuzzy |
i think time/age alone is a bad measure tho. you could be too quick in killing PRs that deserve atention |
| 00:07 |
MTDiscord |
<Jonathon> also some inactivity comes back to wuzzys point of lack of official review |
| 00:07 |
Wuzzy |
exactly! I bet many contributors are extremely frustrated by this. |
| 00:08 |
Wuzzy |
from their point of view, their hard work has essentially gone to waste |
| 00:08 |
v-rob |
A lack of support from other people is a good reason for a potential close, e.g. I had a bunch of formspec PRs that almost no one seemed to care about, so I closed them. |
| 00:08 |
Wuzzy |
new close reason: "poor/questinable design" |
| 00:08 |
Wuzzy |
(implying it can't be "fixed" with a few line changes) |
| 00:10 |
Wuzzy |
i have a new idea for a close reason: "merge nightmare". that is, when a PR is so old and dusty and incompatible, that if you would merge it today, you would have to solve a TON of conflicts first |
| 00:10 |
v-rob |
Yes, Minetest has too many features that would have been much better if they had been implemented better, so we shouldn't merge things that aren't future proof. See: formspecs and things like them |
| 00:12 |
Wuzzy |
hmm. in general we should be more bold in actually DECIDING on what to do with a particular. support or rejection should be given. |
| 00:12 |
Wuzzy |
because the situatoin right now is that we have 100 open i.e. UNDECIDED prs |
| 00:13 |
MTDiscord |
<Jonathon> irronically the oldest PR is marked for 5.5.0 |
| 00:13 |
Wuzzy |
is there a label for "this PR is good/bad"? |
| 00:13 |
v-rob |
Closest is probably "supported by core dev" |
| 00:13 |
Wuzzy |
weirdly absent from PRs tho |
| 00:14 |
Wuzzy |
and triagers would not be allowed to use this label for obvious reasons ? |
| 00:14 |
Wuzzy |
wow, not a single PR is "supported by core dev" |
| 00:14 |
Wuzzy |
Clsoe them all now ? |
| 00:15 |
MTDiscord |
<Jonathon> only one ever has had that label |
| 00:15 |
Wuzzy |
great, it means we only have 1 pr left then ? |
| 00:16 |
v-rob |
Well, a new class of labels like "Bad implementation", "Unnecessary feature", "Inactivity close", and "No support" could be good. |
| 00:16 |
MTDiscord |
<Jonathon> it was merged |
| 00:16 |
v-rob |
Look at that, time to release the final version of Minetest! We're done |
| 00:16 |
Wuzzy |
what is important about such labels is that it should be clear what they mean |
| 00:16 |
Wuzzy |
"unnessary feature" sounds very debatable |
| 00:17 |
v-rob |
Those are just examples |
| 00:19 |
Wuzzy |
here's an idea. go through all old issues (>1 year) that need author attention (meaning they still got "homework" to do). Add "possible close" label and from now they get 1 extra month to respond. if nothing happens, instant close. This does not apply to issues for which the author already did their "homework" |
| 00:19 |
v-rob |
Bug reports exempt |
| 00:19 |
Wuzzy |
oh, i mean PRs only, of course |
| 00:20 |
v-rob |
It would be nice for GitHub to differentiate between feature requests and bugs |
| 00:20 |
v-rob |
Feature requests need cleaning up |
| 00:21 |
Wuzzy |
i feel like the whole labels thing is hot garbage. it is ok for small problems but once you have to deal with >500 issues it just is a big mess |
| 00:23 |
v-rob |
Nearly half (!) of our issues are "Feature request" labeled, while only a fourth are "Bug", and a twelfth is "Unconfirmed bug". That leaves about 200 issues in none of these classifications |
| 00:24 |
rubenwardy |
it was a mild oversight trying to get someone to help with issues who can't labels |
| 00:25 |
rubenwardy |
also: https://github.com/minetest/minetest/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc+-label%3A%22Feature+request%22+-label%3A%22Bug%22+-label%3A%22Unconfirmed+bug%22 |
| 00:25 |
rubenwardy |
regarding v-robs most recent statement |
| 00:25 |
Wuzzy |
lets talkt to microsoft to fix their website. i am sure they will listen to us, who directly compete with one of their core products |
| 00:26 |
rubenwardy |
it's not broken, it just requires javascript |
| 00:26 |
rubenwardy |
and MC isn't one of their core products |
| 00:26 |
Wuzzy |
lol |
| 00:26 |
Wuzzy |
still thinking about "PR close reasons" |
| 00:28 |
Wuzzy |
the newest PR that would fall under my 1 year rule would be #9280 |
| 00:28 |
ShadowBot |
https://github.com/minetest/minetest/issues/9280 -- Code reorg proposal: isolate liquid transformation code by EvidenceBKidscode |
| 00:28 |
v-rob |
Well, we could try to read the obfuscated JavaScript code to find how GitHub adds and removes labels |
| 00:29 |
Wuzzy |
hmm #9280 seems like the perfect close candidate: rebase needed, last post from ANYONE 6 months ago, controversial |
| 00:29 |
ShadowBot |
https://github.com/minetest/minetest/issues/9280 -- Code reorg proposal: isolate liquid transformation code by EvidenceBKidscode |
| 00:31 |
rubenwardy |
3 devs comment interest, no reviews |
| 00:31 |
rubenwardy |
typical |
| 00:32 |
rubenwardy |
_4 devs_ |
| 00:32 |
rubenwardy |
the OP is a core dev now |
| 00:33 |
v-rob |
There should be a differentiation between core developers are interested and core developers are interested enough to review |
| 00:39 |
Wuzzy |
heh |
| 00:40 |
rubenwardy |
in 22 days, these can all be closed https://github.com/minetest/minetest/issues?q=is%3Aopen+label%3A%22Possible+close%22+sort%3Aupdated-asc |
| 00:40 |
rubenwardy |
although tbh the issues can be closed now maybe and reopened if needed |
| 00:40 |
rubenwardy |
the PRs should wait for activity |
| 00:40 |
rubenwardy |
err, should wait the full month |
| 00:41 |
Wuzzy |
yes, of course |
| 00:41 |
Wuzzy |
when i think about it, this "nobody wants to review" is a BIG problem indeed |
| 00:41 |
Wuzzy |
more than labels |
| 00:42 |
Wuzzy |
i think labels are not really our main problem. they are annoying but not the end of hte world |
| 00:42 |
v-rob |
Honestly, closing issues and pull requests just needs more aggression than anything else. |
| 00:42 |
Wuzzy |
any guidelines for potential reviewers? tbh i dont really know where to start |
| 00:42 |
v-rob |
Reviewing is a problem that no one has found a potential solution to. |
| 00:42 |
Wuzzy |
well often you dont know if a pr is good or bad if u havent read the code, so ? |
| 00:43 |
rubenwardy |
I'd google "how to perform code reviews" |
| 00:43 |
Wuzzy |
no i mean more mintest-specific |
| 00:43 |
Wuzzy |
anything special i need to know? |
| 00:43 |
MTDiscord |
<Jonathon> https://github.com/minetest/minetest/issues/420 supported by core dev yet possible close? isnt that conflicting....? |
| 00:44 |
rubenwardy |
because I'm pretty sure that's fixed |
| 00:44 |
MTDiscord |
<Jonathon> ah |
| 00:44 |
rubenwardy |
there was a PR to kick players before shutdown |
| 00:44 |
rubenwardy |
I just need to test it |
| 00:45 |
Wuzzy |
what is your workflow for actually testing a PR? |
| 00:46 |
v-rob |
Wait until someone requests my review, then find some time, look through the code, test it, and approve/request changes |
| 00:48 |
rubenwardy |
I read the description. I glance through the code. If it's a bug, I try to reproduce it on master. Then I checkout and test |
| 00:50 |
v-rob |
Anyway, I have to go, but this discussion has given me some motivation to go through some old issues/PRs |
| 00:51 |
Wuzzy |
no i meant what git line u use for the "checkout" part? |
| 00:51 |
Wuzzy |
i know it is 'git pull' but what if the pr is months old? |
| 00:52 |
Wuzzy |
so i imagine it works like this: git checkout <old commit on which branch is based on>; git branch <new_branch_name>; git checkout <new_branch_name>; git pull <repo> <new_branch_name> ... |
| 00:52 |
Wuzzy |
this seems like a mouthful, isn't there a faster way to do this? ? |
| 00:53 |
rubenwardy |
I use the git hub cli tool. It's MIT licensed and I can do git checkout https://github.com/minetest/minetest/pulls/123 |
| 00:53 |
Wuzzy |
oh, there is a tool?? |
| 00:53 |
rubenwardy |
there's also GitHub CLI, you might be able to make PRs and do labels with that |
| 00:54 |
Wuzzy |
is it "hub"? |
| 00:54 |
rubenwardy |
https://hub.github.com/ https://cli.github.com/ |
| 00:54 |
Wuzzy |
oh i totally forgot about that |
| 00:54 |
Wuzzy |
ah, that is actually really nice. i should look at it |
| 00:54 |
rubenwardy |
I think other devs use git am or other stuff |
| 00:54 |
Wuzzy |
this tool must be from the days before MS took over, I suppose ? |
| 00:55 |
Wuzzy |
do you actually use hub? |
| 00:56 |
rubenwardy |
I use hub |
| 00:56 |
Wuzzy |
i think i heard of it before, i just completely forgot about it, lol |
| 00:57 |
rubenwardy |
GitHub CLI is from after MS and it's also MIT |
| 00:57 |
rubenwardy |
I think |
| 00:57 |
|
kilbith_ joined #minetest-dev |
| 01:04 |
Wuzzy |
GitHub CLI is "gh" on the console |
| 01:05 |
Wuzzy |
gh is hg backwards. curious ... mercurialliminati confirmed! |
| 02:31 |
Wuzzy |
I have a request: When 5.4.0, I ask you to priotize the review/merge of #10693 over all other builtin-related issues. this PR touches many files, so is a conflict-magnet. the earlier this PR is merged, the better, as it reduced conflict pain |
| 02:31 |
ShadowBot |
https://github.com/minetest/minetest/issues/10693 -- Builtin translate (2nd attempt) by Wuzzy2 |
| 02:31 |
Wuzzy |
I mean *after 5.4.0 released* of course. |
| 03:44 |
pgimeno |
Wuzzy: I normally add the remote, then fetch the branch and checkout: git remote add user_name repo_url ; git fetch user_name; git checkout user_name/branch_name. No need for GH-specific stuff. |
| 03:45 |
Wuzzy |
but i already fell in love with the other program. works nicely |
| 03:45 |
Wuzzy |
? |
| 03:45 |
pgimeno |
kk |
| 03:47 |
pgimeno |
just keep in mind that then you're locked-in on GH |
| 04:05 |
rubenwardy |
It's such a small feature that I'd be surprised if other major sites don't have their own version |
| 04:06 |
rubenwardy |
It's no more lock-in then using any other piece of software or GitHub itself - you get used to doing something in a certain way, but no data is locked in |
| 04:07 |
rubenwardy |
I also use GitLab for all my personal projects :P |
| 05:00 |
|
MTDiscord joined #minetest-dev |
| 05:51 |
|
olliy joined #minetest-dev |
| 07:45 |
|
mizux joined #minetest-dev |
| 08:00 |
|
ShadowNinja joined #minetest-dev |
| 08:35 |
|
vesper11 joined #minetest-dev |
| 09:58 |
|
Icedream joined #minetest-dev |
| 10:18 |
|
Icedream joined #minetest-dev |
| 10:19 |
|
calcul0n_ joined #minetest-dev |
| 10:25 |
Krock |
!tell kilbith table.indexof({ [-1] =" value" }, "value") -> -1. same as the error case. how would that be solved? |
| 10:25 |
ShadowBot |
Krock: An error has occurred and has been logged. Please contact this bot's administrator for more information. |
| 10:46 |
MTDiscord |
<appguru> Krock: Why not return nil if not found? |
| 10:58 |
Krock |
that's not according to the documentation |
| 11:04 |
MTDiscord |
<appguru> Yeah, but that'd be a solution |
| 11:04 |
MTDiscord |
<appguru> If you want backwards compatibility at all cost, return -1 if not found, and use a second return value to indicate whether it was actually not found or key is -1 |
| 12:05 |
|
calcul0n__ joined #minetest-dev |
| 12:35 |
sfan5 |
pushing fix for #10920 in a few mins |
| 12:35 |
ShadowBot |
https://github.com/minetest/minetest/issues/10920 -- Segfault on quit |
| 12:35 |
|
Fixer joined #minetest-dev |
| 12:41 |
sfan5 |
merging #10921, #10922 in 5m |
| 12:41 |
ShadowBot |
https://github.com/minetest/minetest/issues/10921 -- Reduce ore noise_parms error to deprecation warning by rubenwardy |
| 12:41 |
ShadowBot |
https://github.com/minetest/minetest/issues/10922 -- Fall back to default when rendering mode (3d_mode) is set invalid by srifqi |
| 12:51 |
|
Fixer_ joined #minetest-dev |
| 13:36 |
sfan5 |
* If `continuous` is true, the Lua entity will not be moved to the current |
| 13:36 |
sfan5 |
position before starting the interpolated move. |
| 13:36 |
sfan5 |
wtf does this mean |
| 13:39 |
sfan5 |
ok it kinda makes sense now |
| 13:40 |
sfan5 |
looking at ::moveTo() and seeing if(!continuous) sendPosition(true, true); is a bit confusing |
| 13:40 |
sfan5 |
since at the next step setPosition will be called anyway |
| 13:40 |
sfan5 |
the last paramter apparently makes a difference clientside |
| 13:40 |
sfan5 |
parameter* |
| 13:58 |
sfan5 |
merging game#2816 soon |
| 13:58 |
ShadowBot |
https://github.com/minetest/minetest_game/issues/2816 -- Update translation templates by Wuzzy2 |
| 13:59 |
sfan5 |
and game#2820 as soon as I've tested it (5m) |
| 13:59 |
ShadowBot |
https://github.com/minetest/minetest_game/issues/2820 -- Smoothen lava sounds at loop points by An0n3m0us |
| 13:59 |
sfan5 |
or wait no not that one yet |
| 14:17 |
|
proller joined #minetest-dev |
| 14:42 |
|
kilbith joined #minetest-dev |
| 14:55 |
pgimeno |
tested #10742, it works as expected. I barely understand the code, so I can't tell for sure if the fix is correct, but qualitatively it looks correct. |
| 14:55 |
ShadowBot |
https://github.com/minetest/minetest/issues/10742 -- Fix world-aligned node rendering at bottom by Wuzzy2 |
| 14:56 |
pgimeno |
It's also extremely low-risk. It doesn't affect anything other than the bottom face of world-aligned tiled nodes, which was working wrong. |
| 15:33 |
sfan5 |
game#2822 |
| 15:33 |
ShadowBot |
https://github.com/minetest/minetest_game/issues/2822 -- [NO SQUASH] Maintenance PR by sfan5 |
| 15:49 |
|
Fixer joined #minetest-dev |
| 15:52 |
|
kilbith joined #minetest-dev |
| 16:15 |
|
numzero joined #minetest-dev |
| 16:29 |
|
Andrey01 joined #minetest-dev |
| 16:30 |
Andrey01 |
the core devs, could you please consider #10909? |
| 16:30 |
ShadowBot |
https://github.com/minetest/minetest/issues/10909 -- [Entities] Add properties setting automatic box rotation. by Andrey2470T |
| 16:30 |
Andrey01 |
and also attach the corresponding labels to that |
| 17:13 |
|
Taoki joined #minetest-dev |
| 17:21 |
|
Taoki joined #minetest-dev |
| 17:27 |
|
kilbith joined #minetest-dev |
| 18:29 |
Andrey01 |
I`ve recreated #10924 |
| 18:29 |
ShadowBot |
https://github.com/minetest/minetest/issues/10924 -- [Reset] Add modslist formspec for /mods command. by Andrey2470T |
| 18:29 |
MTDiscord |
<srinivas> would it be possible to make param2 colorisation have 24 bits instead of 8? It will give people full hex code coverage, instead of a far far limited one? |
| 18:30 |
Andrey01 |
now the formspec appears on calling of /mods command, not /help mods |
| 18:30 |
sfan5 |
@srinivas param2 only has 8 bits so you can't just do that |
| 18:31 |
MTDiscord |
<srinivas> Ah i see, thanks |
| 18:31 |
MTDiscord |
<srinivas> is it possible to increase its sixe? or are the costs not worth it? |
| 18:31 |
MTDiscord |
<srinivas> *size |
| 18:33 |
sfan5 |
it's not a simple process |
| 18:35 |
Krock |
plus 1 additional byte per loaded mapnode |
| 18:38 |
MTDiscord |
<srinivas> ah i see |
| 18:38 |
MTDiscord |
<srinivas> also, 1? i counted 2? |
| 18:40 |
Krock |
one byte for hue, the other for birghtness |
| 18:40 |
Krock |
you could also split RGB but that would indeed require 2 additional bytes |
| 18:41 |
MTDiscord |
<srinivas> ah |
| 18:41 |
MTDiscord |
<srinivas> and Saturation? |
| 18:42 |
MTDiscord |
<srinivas> i always thout it was Hue Saturation Luminosity/brightness? |
| 18:42 |
sfan5 |
if you had 16-bits you could do rgb565 |
| 18:42 |
sfan5 |
or hsv565 if that's somehow better |
| 19:16 |
|
fluxflux joined #minetest-dev |
| 19:24 |
|
fluxflux_ joined #minetest-dev |
| 19:37 |
|
Wuzzy joined #minetest-dev |
| 19:46 |
|
kilbith joined #minetest-dev |
| 19:50 |
|
kilbith joined #minetest-dev |
| 19:56 |
|
Andrey01 joined #minetest-dev |
| 20:41 |
numzero |
+4 bytes, not 2 |
| 20:41 |
numzero |
due to alignment |
| 20:42 |
numzero |
but these 2 excess bytes could be used for RGB lighting |
| 20:42 |
numzero |
or whatever else |
| 21:01 |
MTDiscord |
<exe_virus> Hate to bring this up, but are the mtg compression PRs going to be merged before 5.4 release? They were in a while ago and both are up to date with master... |
| 21:04 |
sfan5 |
I can guarantee that you will need to rebase it again because the maintenance PR needs to go in before the release |
| 21:06 |
sfan5 |
#2807 is fine to merge though |
| 21:06 |
ShadowBot |
https://github.com/minetest/minetest/issues/2807 -- Android: use loop inside makefile to spare repetition by est31 |
| 21:06 |
sfan5 |
game#2807 obv |
| 21:06 |
ShadowBot |
https://github.com/minetest/minetest_game/issues/2807 -- Compress all .obj files by ExeVirus |
| 21:06 |
|
Fixer joined #minetest-dev |
| 21:12 |
sfan5 |
if anyone has some time spare #10905 is an easy issue to preprare a PR for |
| 21:12 |
ShadowBot |
https://github.com/minetest/minetest/issues/10905 -- Reduce "Empty translation" debug log message level from error to info or action |
| 21:12 |
sfan5 |
or #10911 |
| 21:12 |
ShadowBot |
https://github.com/minetest/minetest/issues/10911 -- get_short_description's description finding algorithm is flawed |
| 21:25 |
|
kilbith joined #minetest-dev |
| 21:51 |
|
kilbith joined #minetest-dev |
| 22:03 |
ronoaldo |
Any pre-built android binaries? |
| 22:20 |
MTDiscord |
<exe_virus> @rubenwardy hey I'm far from internet this weekend, can barely send this message, can you or someone else update the MTG compression PR by moving the script to the util folder? Or just let me do it in another PR later when I'm back at civilization? |
| 22:21 |
MTDiscord |
<exe_virus> Also, .obj PR can't be fixed until Monday for me, sorry guys. I'm tryin to keep everything up to date |
| 22:26 |
sfan5 |
sure that's fine |
