Time |
Nick |
Message |
01:05 |
|
sofar joined #minetest-dev |
02:30 |
|
Cornelia joined #minetest-dev |
02:48 |
|
MayeulC joined #minetest-dev |
04:33 |
|
MayeulC joined #minetest-dev |
04:40 |
|
jas_ joined #minetest-dev |
09:13 |
|
YuGiOhJCJ joined #minetest-dev |
09:14 |
|
Wuzzy joined #minetest-dev |
10:25 |
|
Lunatrius joined #minetest-dev |
11:13 |
|
T4im joined #minetest-dev |
11:18 |
|
Fixer joined #minetest-dev |
11:21 |
|
Fixer_ joined #minetest-dev |
11:26 |
|
calcul0n joined #minetest-dev |
11:55 |
|
Gael-de-Sailly joined #minetest-dev |
12:00 |
crazyR |
I'm considering paying a C++ developer to take care of some of the requests.... is there any way to determine which requests have been approved and are waiting for someone to implement them? |
12:00 |
crazyR |
I ask this because i dont want to spend money on somthing that will only end up not being implemented for one reason or another |
12:11 |
crazyR |
looking through the issues/pull requests... i can see there is tags for approval of PR's could there possibly be tags to mark feature requests as approved/awaiting development etc? |
12:18 |
rubenwardy |
if there's no developer comments in the issue, then you'll probably have to ask here |
12:19 |
rubenwardy |
is this a bounty thing or a hiring someone thing? |
12:50 |
crazyR |
I'm looking to hire people on a job by job basis. each month aiming to tackle 1 or 2 requests.. |
12:51 |
|
Strayko joined #minetest-dev |
13:45 |
jas_ |
i apologize but how can there be any guarantee? |
13:59 |
crazyR |
guarantee of what? |
13:59 |
crazyR |
jas_^ |
14:03 |
jas_ |
oh sorry you said u didnt want to spend money unless u knew for sure it'd make it thru? |
14:04 |
jas_ |
i'm just curious really how it would work. paying for features aounds like fun! |
14:20 |
crazyR |
basically.... the devs decide what should be approved vs what should not be approved in the same way they do now... only, instead of putting an idea to the back of the priority due to its importance... they tag it with some think like "approved - Needs Development" then myself and anyone else who might want to spend pennies can come along and decide to pay someone to make/fix/implement it. |
14:20 |
crazyR |
bounties are great but means waiting for someone to pick it up and do it.... but actually hiring someone todo it means it will get done in a timely manner |
14:57 |
|
YuGiOhJCJ joined #minetest-dev |
15:14 |
|
Jordach joined #minetest-dev |
15:24 |
|
Ruslan1 joined #minetest-dev |
15:47 |
|
Cornelia joined #minetest-dev |
16:23 |
nerzhul |
crazyR waht PR did you want ? |
16:25 |
|
T4im_ joined #minetest-dev |
16:25 |
crazyR |
nothing paticular... |
16:31 |
nerzhul |
difficult then :D |
16:36 |
crazyR |
not really. im basically just asking the developers/reviewers to make it clear when they approve of a feature(even when they feel nobody can currently take ownership of it :D - I've noticed over the years that a lot of good feature requests that get favourable comments from the devs/reviewers end up going stale when there isn't enough time for it to be done at that time. |
16:36 |
crazyR |
if that makes any sense :/ |
16:42 |
Shara |
I'd really love to see a proper way of handling concept approvals so people can clearly judge what might be easy to get accepted if only they code it well. |
16:44 |
crazyR |
yeah there is that as well.. if unofficial developers/contributors knew that something they was about to work ok would lamost definiatly be implemented. i feel it would encourage them to go at it. |
16:44 |
crazyR |
*almost |
16:51 |
|
T4im joined #minetest-dev |
17:23 |
exio4 |
it also depends on where the programmer is, for starters, somebody is switzerland is likely out of question :P |
17:30 |
|
Gael-de-Sailly joined #minetest-dev |
17:47 |
|
paramat joined #minetest-dev |
17:50 |
paramat |
it might be useful anyway to have '1/2 concept approval(s)' labels. currently to gauge core dev support for a feature request we have to read through each thread (and there are hundreds). then something is considered 'concept approved' if 2 core devs do so |
17:53 |
paramat |
also 2 thoughts: 1. the best person to hire is an existing core dev or existing talented contributor, as we know the code. 2. (related) someone new from outside the project may not be familiar with MT so not be a good choice |
18:02 |
paramat |
.. so will find it a much harder job |
18:02 |
rubenwardy |
Someone outside the project may have better skills |
18:02 |
rubenwardy |
For example, with graphics |
18:06 |
crazyR |
the way i see it. if i hire someone (from outside or inside the community) regardless as to how familiar they are with the project. once they agree to it. then they must complete it to a satisfactory standard. if not they wont get paid. on that basis it is safe to allow the hired developer to decide if he or she is capable of doing the job. that being said im not against paying a inside dev to do jobs. |
18:07 |
crazyR |
in any case we do need to have some clarity on what features have an extremely high likelihood of being accepted. |
18:10 |
p_gimeno |
hm, reviewing the work that person will do will also take developers time |
18:11 |
p_gimeno |
it's not that different to a regular PR |
18:13 |
paramat |
there's a danger no core dev has time to review, which happens a lot recently. we're not lacking contributions at all, mostly review time |
18:13 |
p_gimeno |
yeah, that's where I was going |
18:15 |
crazyR |
out of curiosity.... roughly how many pr's could one dev review in a 2 hour period? |
18:15 |
paramat |
ha |
18:15 |
paramat |
=) |
18:15 |
T4im |
inb4 "pay to get your contribution reviewed" |
18:15 |
paramat |
depends, 2-0.5 |
18:16 |
paramat |
or even 0.2 |
18:16 |
crazyR |
T4im hahaha i like your thinking :D |
18:16 |
crazyR |
is there anything that could speed the proccess up? |
18:16 |
paramat |
core devs often don't have time however much you pay them |
18:17 |
T4im |
review of #4343 took so far >2 years and seems still to be going on :p |
18:17 |
ShadowBot |
https://github.com/minetest/minetest/issues/4343 -- Profiler: Better structures, count calls, measure relative usage by t4im |
18:17 |
T4im |
i don't even think it's the oldest one :x |
18:18 |
paramat |
yeah it's tricky because usually money isn't the issue, it's time, which often can't be bought in someone's life |
18:19 |
crazyR |
so maybe there needs to be an active Dev recruitment proccess? thats not to say give anyone and everyone dev status... but somehow we do need to find a way of increasing the manpower.... |
18:19 |
T4im |
perhaps it would help you to have your core devs concentrate on where the project is going and open yourself up to trustworthy contributors to join the reviewboard on technical things |
18:20 |
paramat |
we already are recruiting :) |
18:20 |
crazyR |
^ that i agree on... i think having a review board seperate to core devs would freeup more time for core devs. |
18:20 |
T4im |
like.. if the core-devery has already decided to adopt a feature, why shouldn't p_gimeno's voice count as an approval on the technical merit of it? |
18:20 |
T4im |
just as one example |
18:21 |
paramat |
we already allow talented contributors to review, for example numberzero is a huge help with graphics |
18:21 |
crazyR |
How many reviewers are there? |
18:23 |
T4im |
if numberzero approves it, does it count as approval? |
18:23 |
paramat |
core devs already take into account trusted reviews and can convert that into a contribution towards their own +1 |
18:23 |
paramat |
well, not officially |
18:23 |
crazyR |
paramat but that still doesn't remove the pressure from the core devs.... which partly defeats the object... |
18:23 |
paramat |
at that point he might as well be a core dev |
18:24 |
crazyR |
brb 10 mins |
18:24 |
T4im |
well that is an issue then; because from what i've seen, even though you take certain people's opinion into account, there seems to be a significant hesitation, and an understandable one, to approve something just based on that |
18:25 |
T4im |
like "if i approve this because someone else reviewed it, it's my responsibility, so i better review it myself as well.. in 2 years" |
18:25 |
paramat |
essentially, someone has to be a core dev to have approval power, there's no getting around that ) |
18:26 |
paramat |
:) |
18:26 |
rubenwardy |
There aren't any good core dev candidates |
18:26 |
|
Krock joined #minetest-dev |
18:26 |
rubenwardy |
Numberzero already declined |
18:26 |
paramat |
T4im yes |
18:28 |
paramat |
so yeah it's a very difficult situation |
18:28 |
p_gimeno |
maybe pay someone to train them to be a core dev? |
18:28 |
Krock |
oh! #4343 has new commits |
18:29 |
ShadowBot |
https://github.com/minetest/minetest/issues/4343 -- Profiler: Better structures, count calls, measure relative usage by t4im |
18:29 |
T4im |
allowing trusted contributors to approve reviews on technical merrits, without being core devs might alleviate your situation; you'd not have to fear letting anyone in you don't deem to be fit, you can try out people where you aren't sure, and you still get reviews going |
18:30 |
paramat |
so, what really needs funding is core devs to review, not more contributions |
18:30 |
T4im |
Krock: just amended with your suggestions :) |
18:30 |
Krock |
I also like to see test results from other contributors - that helps a lot to find issues in the code or to see whether it works as it should |
18:30 |
paramat |
but of course, most core devs may not have time even if you paid |
18:30 |
Krock |
which I really appreciate in the Android PRs |
18:31 |
Krock |
T4im: I see, the changes look good but I didn't notice there were any new |
18:31 |
T4im |
:D |
18:31 |
T4im |
ah ok |
18:31 |
T4im |
thanks :) |
18:31 |
paramat |
"allowing trusted contributors to approve reviews on technical merrits, without being core devs might alleviate your situation" no, they would have to be core devs, and would already be if trusted that much |
18:32 |
T4im |
and why is that? |
18:32 |
Krock |
^ |
18:33 |
paramat |
obvious :) |
18:33 |
Krock |
Helpful reviews, such as code suggestions (not speaking of typos or trivial style issues) are welcome, at least by me |
18:33 |
Shara |
Problem is that trusting someone to review technically already means they must have contributed and worked with MT long and consistently enough to be known to have that competence. So if they'd been around that long and earned that trust, would they not also be trusted to approve concepts appropriately as well, at which point they could just be a core dev? |
18:33 |
Krock |
they can review when they want, but it will still require 2 core dev approvals |
18:36 |
Krock |
.. because there has to be a clear line which separates people whose approvals that count; or not. |
18:36 |
T4im |
Shara: the thing is, that even people known as such are not necessarily becoming core devs; either because they don't want to, or because they can't spend the time that seems to be necessary to become one |
18:36 |
T4im |
or other reasons |
18:36 |
Krock |
well, you're not obligated to do anything as a core dev, but then it's kinda pointless to be one |
18:37 |
T4im |
Krock: yea, i'm essentially suggesting adding an extra line between gameplay and "minetest future" decisions and the sole technical ones of "this piece of code will work as the core devs intend it to" |
18:37 |
Krock |
I should really read this file at least once |
18:39 |
Krock |
^ the one about contributing |
18:39 |
T4im |
hm? :o what file |
18:40 |
Krock |
https://github.com/minetest/minetest/blob/master/CONTRIBUTING.md it's kinda a guide for newcomers. That should also cover the organization questions |
18:42 |
T4im |
:) |
18:42 |
Krock |
T4im: I don't understand what exactly you mean. The code is supposed to work, but apparently core devs form it the way they want it to be? |
18:46 |
T4im |
hm? no, i mean like.. you as a team decide where minetest goes from here; that doesn't contradict delegating technical reviews to contributors |
18:47 |
Krock |
yes, something would go horribly wrong if it were otherwise :) |
18:49 |
T4im |
well right now it is otherwise, right now there is an gatekeeping not just on strategic decisions, but also on quality checking; why would you gatekeep on quality checks, when you have people available that can do that for you? right now it seems a beauracratic process is blockign that from happening |
18:49 |
T4im |
you consult people, but you don't actually give them any power to help you move code review along |
18:50 |
T4im |
which leads on all that work landing back on the gatekeepers |
18:52 |
T4im |
if you are afraid giving individuals from the community full core dev privileges, you are opting now for giving them none, instead of granting limited privileges that might still ease your workload :) |
18:53 |
T4im |
well, they are all just suggestions of course :) |
18:54 |
Krock |
> but you don't actually give them any power to help you move code review along |
18:54 |
Krock |
Maybe I missed it earlier - what would you suggest to partially solve this issue? |
18:54 |
Krock |
what do "limited privileges" include? |
18:54 |
|
kalikatz joined #minetest-dev |
18:55 |
|
kalikatz left #minetest-dev |
18:55 |
p_gimeno |
I've reviewed the technical side of one of Krock's PRs, and Krock reacted to them and fixed most of the issues, so I'm not sure what the point is here |
18:56 |
paramat |
T4im i disagree with the suggestions |
18:56 |
Krock |
yes sure you do <.< |
18:57 |
Krock |
p_gimeno: Heh, I'm glad that someone reviewed the PR who is actually "in the loop" of what's going on, and more important: of how it works |
18:58 |
paramat |
also you make no sense in the last few comments |
18:58 |
T4im |
p_gimeno: so lets say a few core devs look at another pr of someone external, everyone is happy about what it is doing "we need and want that", but they can't find the time to review it; so why shouldn't you be able to approve it for its technical merrits? |
18:58 |
Krock |
Yet it has to be introduced into the engine before 5.0, amybe it needs yet another rebase |
18:59 |
paramat |
anyone trusted to review or check quality would be made a core dev |
19:00 |
T4im |
i wasn't back when you asked me; probably because i have longer periods between reappearing here, that i can't find the time to spend, and i suppose that is a problem |
19:00 |
T4im |
i wouldn't want to anymore either for what it's worth |
19:00 |
p_gimeno |
T4im: has it happened that someone competent has reviewed it on its technical merits and approved it? I believe that's what was said about numberzero earlier in the discussion, and the opinion of the core devs is that it's a big help |
19:01 |
p_gimeno |
IOW, I believe you're painting a situation that doesn't correspond to reality |
19:02 |
p_gimeno |
you need some trust on the competence of the person who reviews, otherwise anyone can just say "hey, this is good, let it in" and cause lots of trouble later |
19:03 |
T4im |
well, of course |
19:03 |
T4im |
are you saying the trust in you to have that competence would be misplaced? |
19:03 |
|
proller joined #minetest-dev |
19:05 |
p_gimeno |
no, I think I proved my competence in that area in the PR when I pointed out the problems, which is what I did. I didn't just say "looks good". |
19:05 |
Krock |
paramat: can I assume you're still neutral on #7727? |
19:05 |
ShadowBot |
https://github.com/minetest/minetest/issues/7727 -- New sneak: Smoothen the climb up event by SmallJoker |
19:07 |
T4im |
indeed, and the same might be true for other people, p_gimeno |
19:08 |
T4im |
you would have some trust on the competence of those people to review correctly or you wouldn't give them this status in the first place |
19:08 |
paramat |
Krock yeah still neutral |
19:09 |
p_gimeno |
T4im: so we're in agreement; but if I went to a PR that is unrelated to an area of expertise where I have shown competence, and just said "This looks good, let it in", then it would just be normal that the developers don't trust my criterion |
19:09 |
paramat |
T4im i think the best we can do is for core devs to allow trusted reviews to contribute to their own giving of a +1, i do this already |
19:10 |
crazyR |
I feel like this whole convo is being brick walled... as per usual.... dont take that to heart.... its just my opinion. |
19:11 |
rubenwardy |
I reviewed PRs way before coming a core dev |
19:11 |
rubenwardy |
Most of the core devs did |
19:13 |
paramat |
crazyR, no, the points are valid, and 'as per usual' is an insult, please don't |
19:13 |
Krock |
development wouldn't go forward only with core dev reviews :3 |
19:13 |
jas_ |
if u pay a core dev u get a fifty percent shot! |
19:13 |
T4im |
indeed, what Krock said :D |
19:13 |
crazyR |
the point of this entire convo started out as finding a way to allow the core developers to do more developing... as apposed to having to do both reviewing and developing... especially when you consider that that paramat says reviewing can take a long time... thats along time of less development or a long time of no reveiwing |
19:14 |
crazyR |
there should be a trusted team of reviewers.... those reviewers answer to developers... its called management. |
19:16 |
T4im |
taking people into that team could be a way for you to check out possible core-dev team candidates as well! |
19:16 |
crazyR |
^ |
19:16 |
T4im |
without having to fully commit to them already |
19:16 |
paramat |
reviewers that can approve need the same trust level as core devs, so would be core devs already, even if all they want to do is review and not direct the project (some are like this already) |
19:17 |
T4im |
you know the approval could just count formally; you don't need to give them actual write access to the repositories or merge privileges |
19:17 |
crazyR |
question. do core developers make mistakes when approving PRs? |
19:18 |
paramat |
we check coredev candidates already on their contributions and reviews |
19:18 |
crazyR |
i would gues thats a yes... and when they do. who is held accountable? its no diffrent. reviewers would have to be trusted and if they made to many mistakes they would be gone. |
19:20 |
Krock |
crazyR: answer: Yes, also when writing and merging PRs. Either a quick amend or late bugfix will follow on that |
19:20 |
Krock |
core devs are still humans, don't worry. |
19:20 |
crazyR |
so it wouldnt be any diffrent if a "reviewer" made a similar mistake? |
19:21 |
Krock |
from how the review importance is currently weighted, it would be less of a mistake since someone else would usually comment on that |
19:23 |
crazyR |
how many devs does it take to get a approval? its 2 or more isnt it? |
19:23 |
rubenwardy |
Finding people who can code review is a lot harder than finding people to concept review |
19:24 |
Krock |
crazyR: to get a PR merged, it requires two dev approvals. Any other dev may also block the PR in case they have major concerns |
19:25 |
Krock |
one of the approval may also come from the PR author (in case they're a dev) |
19:25 |
Krock |
the wiki should also have an entry for this case |
19:25 |
rubenwardy |
Trivial bug fixes only require 1 |
19:25 |
Krock |
oh right |
19:26 |
rubenwardy |
Trivial meaning small and unlikely to cause more issues |
19:26 |
Krock |
.. who is optimally not yourself, but that would work too |
19:27 |
rubenwardy |
https://forum.minetest.net/viewtopic.php?f=7&t=19877 |
19:27 |
Krock |
such as trivial sneak code fixes :D |
19:29 |
paramat |
erm, 1 disapproval can't stop a PR, which is probably wise |
19:29 |
crazyR |
so.... hypothetically lets say we put together a team of 10 reviewers.... and the PR has to have at least 4 reviewer approvals.. once tagged.... a core dev does the merge... the core dev doesnt review the code because that was the job of the reviewers.. the core dev just reviews the feature idea to make sure if conforms to the development direction.... now if a reveiwer approved code x many times that caused a problem they would be |
19:29 |
crazyR |
struck of and replaced.. |
19:30 |
Krock |
paramat: I thought we had some cases in the past where PRs were blocked |
19:30 |
Krock |
would also know a new, actual case |
19:31 |
paramat |
Krock yeah it's an undefined area |
19:31 |
paramat |
any reviewer who can approve needs the same level of trust as a core dev, so would be a core dev already |
19:32 |
rubenwardy |
The only problem this would solve is if you have people good with the code but don't trust their intentions |
19:32 |
Krock |
crazyR: for the current scale of development that looks like a way too sophisticated approach on solving it |
19:32 |
paramat |
and, i feel that quality is more important than dev speed |
19:32 |
crazyR |
how do you learn to trust their intentions... people change all the time. just because you trust them doesnt mean you will always trust them... |
19:33 |
T4im |
rubenwardy: well, intentions, or time commitment, or strategic gameplay decisions, or anything else unrelated to the codequality itself |
19:33 |
Krock |
so according to the time, 5.0 will be a masterpiece |
19:33 |
paramat |
hehe |
19:33 |
rubenwardy |
There's no actual time commitment to being a core dev |
19:33 |
crazyR |
maybe thats half the problem... |
19:34 |
rubenwardy |
Quite a few of the devs haven't reviewed in years |
19:34 |
paramat |
if there was more required commitment some core devs wouldn't be here |
19:34 |
paramat |
because 'life' |
19:35 |
Krock |
no need for those quotation marks btw |
19:35 |
T4im |
well, you know i tend to leave for a year before returning, sometimes two; i believe that disqualified me the last time (unless i'm missing something else, because i'm sure that my code quality wasn't in question back then) |
19:36 |
Krock |
is it now? |
19:36 |
T4im |
i hope not :x |
19:36 |
Krock |
I don't think so. Just barely anyone reviews |
19:37 |
crazyR |
so get rid of themm.... developing minetest doesnt pay(in most cases) but you get a title.... with that title there should be certain commitments.. if said developer can not maintain to the commitments after a period of time. they lose that title untill they can maintian those cimmitments. |
19:37 |
T4im |
:D |
19:37 |
rubenwardy |
I did have you marked as a potential candidate |
19:37 |
rubenwardy |
The amount I do heavily depends on the time of year |
19:38 |
rubenwardy |
For example, I'm more active in the summer and holidays |
19:39 |
T4im |
crazyR: i'd not try to put too much value on the title; it might get a few people to candidate for the title, not the contribution they could do |
19:41 |
Shara |
Given the MT site fails to even update the contributors page each version and is totally out of date, managing who has which title at that level could be... a very fun exercise |
19:41 |
T4im |
:D |
19:41 |
T4im |
hehe |
19:42 |
paramat |
T4im would you be interested in possibly being a core dev in future? |
19:42 |
rubenwardy |
The real big thing that could help with reviewing is having more unit tests and such tools |
19:43 |
rubenwardy |
Heh |
19:43 |
rubenwardy |
Probably too late for that to have as big an impact as it could |
19:44 |
T4im |
paramat: i'm currently recomitting to my former gamedev gig unvanquished; so not anymore; but i could still help out with occasional reviews (and approvals if that would become a thing) on occasion |
19:44 |
paramat |
ok :) |
19:48 |
|
proller joined #minetest-dev |
19:56 |
T4im |
rubenwardy: yea, when i started the integration testing framework back then, the hope was very similar to the conversation today, just more the direction of allowing people to substitute an approval for a unit test :P but that would probably be even more radical of an idea |
19:56 |
T4im |
with an* |
19:56 |
|
Puka joined #minetest-dev |
19:56 |
rubenwardy |
**a unit test |
19:56 |
rubenwardy |
;) |
19:57 |
T4im |
:D |
19:57 |
T4im |
yes :x |
20:04 |
|
proller joined #minetest-dev |
20:17 |
paramat |
nerzhul please could you add #7837 to the 0.4.17.x app as an emergency temporary fix? rubenwardy, sfan5 and Krock support doing this |
20:17 |
ShadowBot |
https://github.com/minetest/minetest/issues/7837 -- Android: Temporarily disable Irrlicht log messages by paramat |
20:45 |
crazyR |
well.... i gues il go back to my corner. :D |
21:37 |
|
Jordach joined #minetest-dev |
22:22 |
paramat |
merging #7816 in 5 mins |
22:22 |
ShadowBot |
https://github.com/minetest/minetest/issues/7816 -- Formspecs: Fix clipped text in multi-line fields by random-geek |
22:26 |
paramat |
merging |
22:28 |
paramat |
done |
23:23 |
|
Ruslan1 joined #minetest-dev |
23:33 |
|
Lunatrius joined #minetest-dev |
23:39 |
paramat |
merging #7727 in 5 mins |
23:40 |
ShadowBot |
https://github.com/minetest/minetest/issues/7727 -- New sneak: Smoothen the climb up event by SmallJoker |
23:46 |
paramat |
merging |
23:51 |
paramat |
actually no, since author requests testing and i haven't |
23:51 |
paramat |
yet |
23:51 |
VanessaE |
also, s/smoothen/smooth/ |
23:52 |
VanessaE |
(or not, meh) |