00:11:30 Stable (0.32) branch on cbro.berotato.org updated to: 0.32.0-9-ga5a68c12b4 05:02:27 Stable (0.32) branch on crawl.akrasiac.org updated to: 0.32.0-9-ga5a68c1 05:08:43 Unstable branch on crawl.akrasiac.org updated to: 0.33-a0-50-g1545d69 (34) 06:45:07 <12g​e0ff> According to https://github.com/crawl/crawl/pulls, DCSS has 2500 closed and 100 open PRs as of today. 06:45:29 <12g​e0ff> So, it's a good time to mention a short article about how one open source project - Bevy, a Rust game engine - handles PRs: https://www.leafwing-studios.com/blog/triage-by-controversy/ 06:45:40 <12g​e0ff> Their key idea was to encourage community reviews, to allow easier and faster processing of small and non-controversial PRs (the article mentions other benefits too). 06:45:49 <12g​e0ff> There was a bunch of Crawl PRs where other contributors left comments/reviews, helping with code and commit conventions, so it's not a completely new thing for DCSS. 06:45:56 <12g​e0ff> Bevy devs just went a few steps further and made such community reviews more official, and it seems to be working for them. So I wonder if this is something DCSS can benefit from too. 08:08:02 <06m​umra> Only 100? That's a whole lot less than on Mantis ... https://crawl.develz.org/mantis/view_all_bug_page.php 08:10:21 <06m​umra> Wait, I just conflated PRs and bug reports didn't it 😂 08:14:59 <06m​umra> It's definitely an interesting idea in the article, and it'd be really good to have things moving a bit more in the PRs. It's probably really discouraging for potential contributors to see PRs that have been hanging around for years without even a comment 08:38:48 <12g​e0ff> Mantis has a few PRs/patches too - 16 are still open - but they're very stale. The only salvageable things there are art submissions. 09:12:40 <06m​umra> It's probably reasonable at this stage to go through the 30-40 oldest PRs (like older than a year) and make a comment that the PR will be closed in a month unless the author indicates they are still interested in following it up. 09:18:37 <06p​leasingfungus> Why? 09:18:40 <04d​racoomega> I think a lot of the very oldest PRs (and quite a few ones in the middle, to be fair) haven't been moved on because various devs that had looked at them over time felt actively uninterested in them being merged at all, not just 'nobody got around to it', and the problem is rarely that they're stale. 09:26:58 <04d​racoomega> Like, I've certainly agreed that our PR process (if we even have a 'process', per se >.>) is inideal, but reading through the linked article, I do sort of feel like there's an inherently considerable more subjective nature to whether a PR is or isn't a good idea for a game than a game engine (which the article is about). Like, there is functional code for functional features that I think would make the game net worse, and it isn't 09:26:58 really a matter of updating the PR in any way. I'm sure I'm not alone in this. If I may theorize, I think there's a decent bit of "Crawl dev looks at PR, Crawl dev thinks PR would be bad for the game (or at the very least, something they would personally dislike to see in it), but doesn't want to personally stick their neck out on the subject without more people on board, or risk starting an argument over something that nobody else is going on move on, 09:26:59 either." and we end up with a bunch of stuff that probably nobody active is keen on, just sittig in limbo. I know there are PRs I've repeatedly thought about closing when I look at them, but worried that would be a little presumptuous of me to do just on my own initiative (and/or invite a debate that I feel I have more important priorities than having at that moment) I do think I ought to be far better at actually saying things to people when the 09:26:59 thing is something as simple as "The thrust of this seems good, but it's too complicated to review quickly and I hope to get to it sometime later." I'm a lot less sure about "The thurst of this seems good, but I strongly dislike how it's written, even if I can't point to any style guideline you're violating" >.>; 09:28:23 <06m​umra> I mean it's very easy to pick an example at random: https://github.com/crawl/crawl/pull/2437 Here the author seems to have lost interest as there were multiple rounds of feedback and no further response from the author for some time. Saying "we'll close this if there's no further interest" is a normal course of action (in my experience of other projects) 09:30:47 <06p​leasingfungus> it seems like we should close PRs that we think are unlikely to ever be merged 09:31:10 <06p​leasingfungus> idk if a ping for ‘further interest’ is necessary or helpful 09:31:53 <06p​leasingfungus> like, we can just close that pr and say ‘sorry, this doesn’t seem to have panned out, thanks for working on it!’ 09:33:33 <06p​leasingfungus> contrarily, i don’t see a need to close, say, https://github.com/crawl/crawl/pull/2456; i’d still like us to merge some version of it at some point 09:33:58 <06m​umra> Yeah this is definitely what I felt was happening as well 09:38:01 <09g​ammafunk> god I can't ever see iood shortbow ever be not highly annoying to use in practice, but I guess if you tweak the bow's iood's enough it might work 09:40:05 <09g​ammafunk> I also agree that it's somewhat different working on a game engine compared to working on an actual game. Content contributions for games are probably more likely to be fundamentally a bad idea to add to the game compared to that for a game engine 09:40:47 <06m​umra> Maybe a label like "of interest" or something so there's at least some indication it could get into the game but needs more work 09:41:00 <09g​ammafunk> maybe this article isn't really talking about the rate of PR approval though 09:41:15 <09g​ammafunk> or the chance of approval, I should say 09:42:07 <06m​umra> Then it's easier to go and cull stuff that nobody is interested in and has been stale for ages. And a less bloated PR list would encourage people a bit more to go and weigh in on stuff that is interesting 09:42:23 <04d​racoomega> Like, I've tried to periodically go through the PR backlog (and in some fairness to myself, have merged or otherwise dealt with a moderate number of them) and there are a bunch that I keep passing over every time I look at them as something that I actively have no interest in ever merging myself. Sometimes to the level of "I would argue against this strongly if any other dev actually tried" but also a bunch of "I guess if someone 09:42:23 else really wants to spend their time on this, they could." I wonder a little how many things are secretly in the first category for other people. But it's also been a little unclear to me that leaving PRs in the latter category is actually worse than closing them. Like, limbo is a bad place to be, but saying "Sorry, no, closed." when it's unclear that there's an actual blocker to it isn't great either, is it? 09:43:58 personally, I would prefer there to be a comment to that extent, and possibly a label 09:44:28 also, cabal has "status: consider closing" 09:44:45 which can be thought of as a "soft close" 09:45:25 <06m​umra> Which is why I thought "we'll close this soon if there's no further movement" at least gives other devs a chance to say something 09:45:38 <06m​umra> Yeah basically this 10:06:16 <04d​racoomega> A label for which? "I don't want to facilitate this (but maybe someone else will show up who will?)" doesn't seem labelable. A process for "I think this is an actively bad idea" might be possible, though I'm not exactly sure how best to go about it with the kind of lack of formality Crawl dev has had for ages. (On a personal level, I admit to often being uncomfortable to come right out and say that about something no other dev seems 10:06:16 to even be advocating for. >.>) 10:14:49 <06m​umra> i think "worth exploring more" vs "not interested / actively bad" is a good distinction to at least kick off a triage process 10:16:13 <06m​umra> leaving things to hang forever with no closure is probably not a good outcome (especially for, say, a first time contributor) 10:22:31 <06m​umra> The very oldest PR, already had such a comment 🙂 https://github.com/crawl/crawl/pull/1059 Seems safe to just close 10:22:31 <06m​umra> https://cdn.discordapp.com/attachments/747522859361894521/1281665850335826045/image.png?ex=66dc8bd6&is=66db3a56&hm=8a7593ffcdf6011651325a5e403b0d3111c8e8a64dc92f0a81ca20f2f47492de& 14:23:14 What's the deal with requesting a password reset?  I can't figure out how 14:42:48 <09g​ammafunk> @Guest11 Which server? You need to contact the server admin 14:43:10 <09g​ammafunk> If it's CAO or CDI, I can help, but for other servers you need to contact the address on the lobby page 14:52:01 CAO 15:00:43 <09g​ammafunk> If you can tell me the username, I can send a password reset link to the on-file email address 15:01:54 It looks like I have saved logins (which don't work) for two accounts, "00gogo00" and "Zach" - I don't know which, if either, has an email address associated with it, but if both do, the latter. 15:09:04 Guest11: password reset link sent to the email on-file for Zach 17:08:30 Hey folks! Was just taking a look at some new open Git issues and saw that Orb of Destruction can get weakened (and I assume affected by other enchanments?) by FulFus clouds. I think there's a real simple fix by adding a `!mons_is_projectile(*mon)` check somewhere, but I saw we're basically calling `bolt::apply_enchantment_to_monster` and so I have 17:08:31 a question: are there ever circumstances where it would make sense for a projectile monster (like an OoD) to be enchanted? 17:10:25 If so, I can simply add the mon_is_projectile check and keep it out of the cloud reaction logic, otherwise, I think it would make sense to add that check and early return within the apply_enchantment_to_monster function itself—although that would affect far more than just changing the FulFus cloud logic. 17:10:49 Git issue in question: https://github.com/crawl/crawl/issues/4010 17:16:19 <04d​racoomega> Given that mons_is_projectile is only true for things that are not interactable in almost any way (and not for foxfires or boulders or multiple other 'projectiles' that intended to be hittable) I think it's reasonably safe to early-out of enchantment beam applications. However that isn't going to affect the more general issue of Fulsome (and some other things) thinking they should warn you about hitting an iood, which is an 17:16:20 issue I already knew about and intended to do a more general refactoring for (since there are multiple other spells not properly respecting this, and it's responsible for a band-aid fix that simultaneously prevents Shatter from hitting living spells), but ran into some issues with the specific implementation of Jiyva's jelly-protecting effects in 0.32 that kept me from getting it done in time for 0.32 17:16:41 <04d​racoomega> (Still plan to try and get this done in early 0.33) 17:19:09 Gotcha, so for this like that and the PR I made in https://github.com/crawl/crawl/pull/3990, I can close that out if there's gonna be a big code revamp already in the works 17:19:15 things like that* 17:20:47 Also, unrelated side-note, since I finally got a code change in (yay!) via https://github.com/crawl/crawl/pull/4006, should I update the credits or ask someone to add me? 17:23:13 <04d​racoomega> Frankly, I probably ought to merge an incremental fix in the short-term, regardless of what more general fix I plan to do in the longer-term. But that's just why I hadn't gotten to it. >.> 17:23:46 <04d​racoomega> How would you like to be credited? 17:24:00 Full name? I think that's the standard 17:24:18 <04d​racoomega> It varies from contributor to contributer. Many prefer pseudonyms, so we like to ask. 17:24:41 I don't mind, since it's nice to get me name on DCSS as an open source project. Put "Alejandro Ramirez" 17:25:07 I can do that short-term fix from FulFus in the meantime 17:26:13 I can tack that onto my current workaround fix for god-protected mons here too: https://github.com/crawl/crawl/pull/3990 17:33:10 <04d​racoomega> There you go ^^; 17:33:34 Graci 17:33:40 03DracoOmega02 07* 0.33-a0-51-ge99af74eb8: Add Alejandro Ramirez to CREDITS.txt 10(4 minutes ago, 1 file, 1+ 0-) 13https://github.com/crawl/crawl/commit/e99af74eb864 17:33:41 03DracoOmega02 07[stone_soup-0.32] * 0.32.0-10-g6daa67bc45: Add Alejandro Ramirez to CREDITS.txt 10(4 minutes ago, 1 file, 1+ 0-) 13https://github.com/crawl/crawl/commit/6daa67bc4554 22:07:27 Stable (0.32) branch on underhound.eu updated to: 0.32.0-10-g6daa67bc45 22:35:39 Unstable branch on crawl.develz.org updated to: 0.33-a0-51-ge99af74eb8 (34) 22:59:17 Windows builds of master branch on crawl.develz.org updated to: 0.33-a0-51-ge99af74eb8 23:13:20 Unstable branch on cbro.berotato.org updated to: 0.33-a0-51-ge99af74eb8 (34) 23:56:35 Monster database of master branch on crawl.develz.org updated to: 0.33-a0-51-ge99af74eb8