/tg/Station 13

1 Feature PR open at a time only

Unlimited fix pr's are allowed and I guess we would handle tweaks on a case by case basis dependent on size.


This is to manage the size of the tracker for maintainers (mostly me) as I'm getting overwhelmed by the speed of the tracker.

More importantly it also makes people focus on one pr and get that ready and merged and respond to maintainer feedback straight away.

The atomization rule would still be in place as well.

Yes this slows the rate of change a bit, but I think that it is required.

Sounds like a good idea honestly

I'm in favour, hopefully would lead to people focusing on polishing and finishing existing PRs they're working on over jumping between them

i dunno if that was ever an issue, but yeah

I'm for it

This is dumb

But I understand why

I foresee this calling into debate what exactly is a feature, fix or tweak; mayhaps be good to decide on suitable boundaries for each before this.

Jordie0608 wrote:I foresee this calling into debate what exactly is a feature, fix or tweak; mayhaps be good to decide on suitable boundaries for each before this.

This too

Damn son I guess your limit wasn't much further than mine after all.

100% for this, maybe more. Update 3/10: Mellowed out on this intensely, not sure anymore that it'd [work / be a good thing]

Though it needs an exception for feature pulls that are already feature complete, lest a controversial but finished pull be used as a tool of attrition to punish people by leaving it open forever.

Why not let on more maintainers instead of handling 99% of the PRs by yourself, oranges?

I maintain that oranges isn't actually that project critical, he's just the least tolerant of open pull requests. If he let more things lay open other maintainers would pick up the slack provided the protracted oranges experience hasn't atrophied their merge button abilities too badly. A feature like this would make something like that more practical.

wasn't this already the case for several months
have i been hot rused

There has to be protections in place against intentionally delaying shit. not just by maintainers but by people purposely arguing about shit on a pr to keep it open so the submitter just closes it out of frustration so they can move on.

I forsee this being abused by trolls to keep prs they don't like from ever getting in.

I suggest we make it so it is 1 pr, + another pr for each 3 valid fix prs merged since that 1 pr was opened.

That adds a lot of bookkeeping overhead MrStonedOne

Besides the only people who can really delay a PR are maintainers

Actually wait a minute none of this is enforceable at all.

oranges wrote:That adds a lot of bookkeeping overhead MrStonedOne

Besides the only people who can really delay a PR are maintainers

yes but maintainers don't want to take the fall for a PR that seems unpopular because 3 people spammed 200 comments

Incoming wrote:I maintain that oranges isn't actually that project critical, he's just the least tolerant of open pull requests. If he let more things lay open other maintainers would pick up the slack provided the protracted oranges experience hasn't atrophied their merge button abilities too badly. A feature like this would make something like that more practical.

Nope, most maintainers don't touch big feature PRs because of my above post

A fairly high upkeep solution would be no new features while one of your existing features has an outstanding issue. This would require an extreme amount of tagging and usage of projects on every issue, plus a lot of memorization and shit. Probably not feasible, but maybe good to keep in mind in an informal sense.

it would also disqualify 99% of our developersform making any more changes

As long as my feature PRs don't sit unreviewed for a week i'm on board with this

No support.

If the reason for this rule is because the maintainers cannot keep up with the issue tracker, then they should hire additional high-activity, objective maintainers (Cheri, Kor, and RR are excellent models for what a maintainer should be! MSO as well, even though he is not a maintainer). If a coder wishes to make several loosely related changes to the same thing, but the changes are sufficiently different to require atomization, it means that he has to split up his PRs and wait for the first PR to be reviewed before he can open the second.

This especially may hurt coders who work on large projects which require several days or more for development. If such a coder wants to make a non-bug related change to another feature of the game, he is constrained for a rather long time. This might not even be his fault, because maintainers are not always timely in their reviews - especially for larger PRs.

I feel that this would hurt development more than it would improve it. The soft-freeze rule of "One feature One Fix" would be better suited to forcing coders to focus on fixes without locking them down in some cases.

TL;DR: Do not slow down the coders. Add more maintainers to handle them.

Gun Hog wrote: If the reason for this rule is because the maintainers cannot keep up with the issue tracker, then they should hire additional high-activity, objective maintainers (Cheri, Kor, and RR are excellent models for what a maintainer should be! MSO as well, even though he is not a maintainer). If a coder wishes to make several loosely related changes to the same thing, but the changes are sufficiently different to require atomization, it means that he has to split up his PRs and wait for the first PR to be reviewed before he can open the second.

Better than having like 5 controversial PRs open at once and have them pushing the shit PRs towards merging at all costs

This especially may hurt coders who work on large projects which require several days or more for development. If such a coder wants to make a non-bug related change to another feature of the game, he is constrained for a rather long time. This might not even be his fault, because maintainers are not always timely in their reviews - especially for larger PRs.

Don't open your PR or open and close it? It's really not that hard to manage open PRs and Closed PRs and still manage your code workflow.

I feel that this would hurt development more than it would improve it. The soft-freeze rule of "One feature One Fix" would be better suited to forcing coders to focus on fixes without locking them down in some cases.

Didn't really work out super well the last time, why would it be better any other time around? Really was just "meh" status.

My new opinion is that this is a good thing that coders should strive to maintain personally, but trying to implement it as policy seems really hard and/or just doomed to piss people off for very little gain.

More maintainers might help, but that' should be tied to maintainers splitting Pull Requests more evenly between them casually.
I'd probably take up a maintainer position these days if asked. Probably wouldn't be very aggressive in the merge game though.

Gun Hog wrote:If the reason for this rule is because the maintainers cannot keep up with the issue tracker, then they should hire additional high-activity, objective maintainers (Cheri, Kor, and RR are excellent models for what a maintainer should be! MSO as well, even though he is not a maintainer)..

None of three people you mentioned are approaching anything like a reasonable merge rate to keep up the with the workload.

Anturk and Razharas are the only ones who are even semi active in merging prs.

oranges is like the ghost in spelunky

if u dont finish up quick he's gunna git ya

Well I mean there's more to maintaining something than just merging everything. In some ways more merging promotes more development, people see a short turn around time and lower standards and they get excited. If it's getting too frantic to handle adding more heavy merging people is really just a temporary solution. Maybe the solution is to get more selective so people cool off a bit and put more thought into what they're proposing.

what do you think this thread is for

oranges wrote:

Gun Hog wrote:If the reason for this rule is because the maintainers cannot keep up with the issue tracker, then they should hire additional high-activity, objective maintainers (Cheri, Kor, and RR are excellent models for what a maintainer should be! MSO as well, even though he is not a maintainer)..

None of three people you mentioned are approaching anything like a reasonable merge rate to keep up the with the workload.

Anturk and Razharas are the only ones who are even semi active in merging prs.

atleast I actually review.

oranges wrote:what do you think this thread is for

Well yeah that's why I brought it up. I agree with the general sentiment you're feeling here.

Even though I'm not really a participant in the development process, I would be concerned as well about this being used to filibuster features.

I'm not sure this is the right solution but its honestly pretty exhausting trying to keep up, especially since tons of PRs lead to bitter arguments.

I want to start aggressively closing PRs that dont have any justification for their existence in the OP as well rather than beg people for the reasoning but last time I tried that other maintainers reopened them.

Jordie0608 wrote:I foresee this calling into debate what exactly is a feature, fix or tweak; mayhaps be good to decide on suitable boundaries for each before this.

Same rules as the feature freeze for what counts as a feature should work okay.

Remie Richards wrote:

oranges wrote:

Gun Hog wrote:If the reason for this rule is because the maintainers cannot keep up with the issue tracker, then they should hire additional high-activity, objective maintainers (Cheri, Kor, and RR are excellent models for what a maintainer should be! MSO as well, even though he is not a maintainer)..

None of three people you mentioned are approaching anything like a reasonable merge rate to keep up the with the workload.

Anturk and Razharas are the only ones who are even semi active in merging prs.

atleast I actually review.

That's cool too, i just want to see you merge more pr's after you review them

I do, Its just i do most of my review on my lunch break these days, so by the time it's merge time I'm on the bus in the middle of no wifi land.
By then someone's usually snapped it up.

There's other cases too, but it really is mostly just other people doing it before me.

Last night before I went to bed I counted 29 open PRs that showed "a day ago" or more recently on github. I know we speedmerged several yesterday as well.

One feature PR open at a time seems like a good idea.

I wouldn't really mind this rule but i think bigger issue is the forever WIP PRs with sap the time of maintainers and creators.

personally i'd hate this because i make shitcode and shitfeatures and have like 3 stale memes open
but in the end this would be a good change to make that stuff not happen :^)

I wouldn't mind this, but the 1 fix = 1 feature solution would work as well, and give coders more freedom to propose multiple features as long as they are willing to do the effort rto fix shit.

I think my solution is the easiest to implement

Bandit's is also a good one that we have tried before with varying success, it still leads to too many feature pr's open imo though.

As it is, doing absolutely nothing is unsustainable.

what if after a feature gets added, no revert/balance PRs by anyone other than the creator for 1-3 week(s) unless said creator drops off the face of the earth during that time

i feel like that idea will lets content creators actually fine tune their work themselves instead of trying to PR their post-launch balance amid 2 revert PRs and 3 more "totally not a revert but makes it useless" prs fighting them

iamgoofball wrote:what if after a feature gets added, no revert/balance PRs by anyone other than the creator for 1-3 week(s) unless said creator drops off the face of the earth during that time

i feel like that idea will lets content creators actually fine tune their work themselves instead of trying to PR their post-launch balance amid 2 revert PRs and 3 more "totally not a revert but makes it useless" prs fighting them

The problem is that the person might have a nice idea, but is horrible in balancing, giving them 3 weeks won't help in anything and might make the feature even worst.

lzimann wrote:

iamgoofball wrote:what if after a feature gets added, no revert/balance PRs by anyone other than the creator for 1-3 week(s) unless said creator drops off the face of the earth during that time

i feel like that idea will lets content creators actually fine tune their work themselves instead of trying to PR their post-launch balance amid 2 revert PRs and 3 more "totally not a revert but makes it useless" prs fighting them

The problem is that the person might have a nice idea, but is horrible in balancing, giving them 3 weeks won't help in anything and might make the feature even worst.

If they want to offload it on another person it's okay, but it lets them develop their vision on their own for a little bit if they need it.

A really bad idea. Bad ideas get shoehorned in sometimes and restricting who can touch it on an open source codebase even for 3 weeks is bad

NikNakFlak wrote:A really bad idea. Bad ideas get shoehorned in sometimes and restricting who can touch it on an open source codebase even for 3 weeks is bad

Let's say you added a new tool for chemists. It's a little OP on launch, and you make a fix PR. Someone makes a revert PR in response to your fix/nerf PR you made, and it gets merged because it has more comments and upvotes because your fix/nerf 1. wasn't tried because all the feedback was "JUST REVERT", and 2. the revert PR has insane support because they don't want to try a fix.

How would you feel, now that a feature you worked hard on got thrown out the window even when you fixed it and they didn't even want to try the fix?

That decision rests with the maintainers who ultimately decide to merge your change or completely remove the retrospective feature. I believe there's a case for that right now with the shadowshrooms deal. Do you have enough faith in your maintainers to properly give things a chance?

Are there any examples of them merging a revert instead of test merging or trying a fix/balance first?

PKPenguin321 wrote:Why not let on more maintainers instead of handling 99% of the PRs by yourself, oranges?

This implies I have a choice in how many pr's I handle.

To me it seems like I wait for someone else to merge it and then the pr is open for many days, no action, I wait and I wait and then at some point I lose patience with waiting and just handle the pr

NikNakFlak wrote:That decision rests with the maintainers who ultimately decide to merge your change or completely remove the retrospective feature. I believe there's a case for that right now with the shadowshrooms deal. Do you have enough faith in your maintainers to properly give things a chance?

Are there any examples of them merging a revert instead of test merging or trying a fix/balance first?

Why did you refuse to answer the question I made and go on a tangent about maintainers?

oranges wrote:

PKPenguin321 wrote:Why not let on more maintainers instead of handling 99% of the PRs by yourself, oranges?

This implies I have a choice in how many pr's I handle.

To me it seems like I wait for someone else to merge it and then the pr is open for many days, no action, I wait and I wait and then at some point I lose patience with waiting and just handle the pr

It's not your onus to keep the pr list short, do what your comfortable doing and if it turns out that the PR page isn't being handled by other people THEN have a discussion about needing more help. Ultimately if one person seems willing to preemptively take on extra "work" most people aren't gonna say "no, don't make my life easier!".

iamgoofball wrote:

NikNakFlak wrote:That decision rests with the maintainers who ultimately decide to merge your change or completely remove the retrospective feature. I believe there's a case for that right now with the shadowshrooms deal. Do you have enough faith in your maintainers to properly give things a chance?

Are there any examples of them merging a revert instead of test merging or trying a fix/balance first?

Why did you refuse to answer the question I made and go on a tangent about maintainers?

Because you are using a bullshit argument and a flawed question.

Let's say you added a new tool for chemists. It's a little OP on launch, and you make a fix PR. Someone makes a revert PR in response to your fix/nerf PR you made, and it gets merged because it has more comments and upvotes because your fix/nerf 1. wasn't tried because all the feedback was "JUST REVERT", and 2. the revert PR has insane support because they don't want to try a fix.

How would you feel, now that a feature you worked hard on got thrown out the window even when you fixed it and they didn't even want to try the fix?

That is my answer. I'm not going to answer your question with "yea I'd feel bad" because I A) Have never had this happen to me and B) don't see this happen in general. If honest to god a maintainer picks a revert over a balance or even worse, a PR that makes a feature work as intended, they are bad maintainers. I don't see this happening however.

Have maintainers even reverted a freshly merged feature before balance changes were attempted? Is there examples of this that you can show? Or are you just saying what-ifs in an attempt to use it as an argument. You are advocating for a "rule" about PRs which probably doesn't even need to exist because there really isn't a problem for it (unless I am mistaken and you can point out examples that I have missed.) I went on that tangent about maintainers, because in reality, they have the power to merge a fix/balance you made or revert it depending on THEIR judgement. The only difference a "3 week grace period does" is protect shitty ideas/balance changes made by the original coder instead of someone else who may be making a more reasonable balance change or even a revert. Maybe if your feature is instantly reverted instead of being balanced, you either need to take a look at the feature itself and wonder why the "big bad maintainers are out to get you and hate your feature" or that you really just suck at balance changes and people agree that someone else changes, or a revert is better. You want to shoehorn in a "policy" about code when as far as I know, doesn't even need to exist. I almost never see features instantly reverted without some sort of balance shifting first if it's new. Most of the time, maintainers and everyone spent so long arguing about something just to get it merged that they would rather see it try to get balanced before reverting. If your feature gets reverted, you must have really fucked up somewhere and any kind of "grace period" is going to change nothing.

Incoming wrote:

oranges wrote:

PKPenguin321 wrote:Why not let on more maintainers instead of handling 99% of the PRs by yourself, oranges?

This implies I have a choice in how many pr's I handle.

To me it seems like I wait for someone else to merge it and then the pr is open for many days, no action, I wait and I wait and then at some point I lose patience with waiting and just handle the pr

It's not your onus to keep the pr list short, do what your comfortable doing and if it turns out that the PR page isn't being handled by other people THEN have a discussion about needing more help. Ultimately if one person seems willing to preemptively take on extra "work" most people aren't gonna say "no, don't make my life easier!".

Because it's a bad faith and rude to people who put their work on our tracker. We owe them a reasonably timely review and I feel embarrassed whenever items stay there for a long item with no activity on our part. It feels like a failure to me.