[SOLVED]Merge Request

https://github.com/T…yCore/pull/6438

This has been sitting for approx. 20 days without any bug reports. I asked on IRC and was told that merges were on hold. Since there have been merges in the past few days, it would appear that this has been lifted. Could someone please merge this? It would be a big help to Hunters and I’m working on more AI cleanup that build off this one anyway.

Thanks

PS: I’m probably going to piss someone off for asking but what other avenue do we have to discuss things like this?

A good commit message plays a big role on what gets pushed first.

Fairly sure that “Merge two commits” does not match the description of the changes.

It’s my experience that devs are lazy slackers that only work well when locked in basements and bribed with pizza and soda. /emoticons/default_tongue.png

Wow, really? Are you serious?

I originally posted the fix with the description:

Fix pet autocast spells such as Furious Howl and Leap

Fixes broken autocast spells such as Furious Howl and Leap to cast when on autocast and “attack” is clicked.

Fixes wrong check for spell cooldown and removes check for duration (some spells with duration such as poison are stackable)

I then made another change and pushed it. I went on IRC and asked for a merge and was told to squash the commits because it would “play a big role on what gets pushed first”. I didn’t feel the need to repost the original description because clearly it is still there (follow the link in my OP).

Don’t get me wrong, I’m not blaming anyone but what exactly do you guys want?

It’s not “there”. What gets pushed is the commit, not the pull request.

I want people to understand that we appreciate contributions a lot but can’t expect to have their code merged in when they want.

Ok, well I thought the original comment would get pushed along with the new “merged” comment. Either way, if a person doing the merge took a minute to look at the thread then they’d see what it fixed.

Right and I don’t usually bitch this much but this one fixes a long-standing bug and has been sitting for 20 days for no reason while the pull request counts have gone from 16 to 19 to 17 to 19 to 16 etc., etc. If I wasn’t a regular around here, I’d say pull requests were being cherry-picked.

If people shouldn’t expect to have pull requests merged when they want then likewise, devs shouldn’t only merge the ones they want. I’m not saying that’s what is happening but to newcomers it could easily be misconstrued to be a ManGOS-esque way of handling contributions.

Pull requests really should be done on a “first in, first out” basis. There are others sitting there even older than mine with really no good reason why they’re not merged yet. Eventually they get so old that they need to be rewritten against the new HEAD but the original author may be long gone.

PS:

I appreciate the dialog here rather than just locking the thread. I know I’m no megamage or raczman but I like to do what I can and a discussion like this helps clear things up for others that may not have the courage to speak up.

By the way, I just updated the comment on the commit so it makes more sense.

MrSmite, I know it may seem pedantic, but in the future could you use our prefix tags in your commits. For example in this case it should have been: Core/Pets: Fixes broken autocast spells such as Furious Howl and Leap to cast when on autocast and “attack” is clicked.

It is just one of those things that makes searching git history for particular fixes easier…

– Brian

Certainly. I usually try to use a prefix but I forgot to in this case.

@Kaelima: Thanks for merging.

— Canned message start

It appears the issue in the original post was solved, so this thread shall be closed. Should you encounter any other difficulties, please open a new thread.

— Canned message end —