/tg/Station 13

Bottom post of the previous page:

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.

oranges wrote:

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.

Counterpoint: No we don't?

This is a volunteer driven open source project with no money involved where the grand majority of developers are at a hobbyist level or lower, No one's entitled to anything here.
Don't beat yourself up over it, we do what we can with the time we're willing to invest into the things we find fun and/or interesting. That's all any of this is.

You hurt the project when you dick around with someone else's personal time. That causes people to leave and get angry. At that point it would literally be better to just automerge things and not have a review process at all since it would do more harm than good.

honestly, the best solution is to just encourage people to self-close their wip things if it ends up taking more time before its ready so they can self-open them once its ready.

When you add the stale tag, specify "I'm going to close this in so and so hours if it has no substantial commit activity, you are encouraged to close it yourself before then so that you can open it yourself when it is ready" or some other shit.

I agree completely with this, but have you considered allowing more feature PRs if they fix something that takes some effort to fix?

Unglobaling this since the other maintainers appear to have no appetite for it

Raising this again because it's still a huge issue in that some days I can merge 21 or more pr's and we still barely stay on top of the tracker.

I would suggest both this + requiring a fix for every feature pr made (developer making a pr is responsible for pointing to the fix pr)

I forgot to answer to this. But it would be reasonable to have this. Last time we attempted it worked out fine in the beginning.

After this pitiful freeze (1000 -> 927 issues even while pruning old, non-issues) I'm going to see about implementing something for this into tgstation-server with a configurable rate limit. We have tons of bugs but no motivation to fix them. Hopefully this gives regular coders the kick in the ass they need in order to do so.

iamgoofball wrote:

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

The only opinions I adhere to are maintainers, headmins, atlanta, and MSO. If I'm not merging it, it either A) sucks in my eye B) failed code review C) failed travis D) looks to complex/big/unknown for me to review it atm. Also the bookkeeping is easy with an automatic database.

What

kevinz000 wrote: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 :^)

Ohhh mmmyyyyyyy whaaaaateeeevvveeerrr willl we dooooo



Real talk, irritating but agreeable, but i would like there to be some room to negotiate however on additional PR's if there is a good enough reason or its related to the first in a directly conjoined way. Get permission in IRC coderbus?

fwoosh you never do shit don't jab at me reeeeee