Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Renaming config for parse_git_dirty() to avoid collision #2323

Merged
merged 1 commit into from Nov 6, 2014

Conversation

michaelorr
Copy link
Contributor

Based on comments on #2270 a previous git config option was overriden.
The git config option for parse_git_dirty() will be renamed to be unique and clearer.

@dcorb
Copy link

dcorb commented Jan 10, 2014

+1

oohlaf added a commit to oohlaf/oh-my-zsh that referenced this pull request Jan 12, 2014
@javache
Copy link

javache commented Oct 27, 2014

Ping?

@mcornella
Copy link
Member

Hi @michaelorr, update your title and description so that it is more clear and succint. Right now the title is too long and spans into the description.

Also, I remember some people wanted this change to be merged, if you can find them and mention them here we'd drive this PR forward with a couple more backers.

@michaelorr
Copy link
Contributor Author

@mcornella I had a PR merged nearly 11 months ago which accidentally broke something. I issued another PR 3 days later to fix it which has sat dormant for 11 months and now you want me to round up people who all agree that not breaking something is the correct way to do it?
Don't get me wrong, I greatly appreciate that you are making an effort to prune old PRs, but this is mind boggling. Open source projects only benefit from community contributions when community contributions are accepted in a reasonable and timely fashion. I would understand if this were a controversial change or required discussion among the core dev team, but wow.

To be honest, at this point, I have written dotfile bootstrapping scripts that rewrite certain parts of omz with sed because I assume they will never be addressed in a proper manner and maintaining a proper fork with the upstream is too much of a pain. I guess I should consider myself lucky that this PR is still able to be merged cleanly.

I will gladly rewrite the title and I will @ping the people who previously commented on this issue but @javache clearly is interested in seeing this merge, so that's at least 2.
@dcorb you still around and care to see this merged?

I apologize for going off on a rant and I realize you are just a maintainer helping out this project in your spare time, but so am I by issuing PRs. I fail to see why having a too long title should be a blocker for a code change which would fix a broken workflow.

@michaelorr michaelorr changed the title accidentally blew away a git config setting used for another purpose, re... Renaming config for parse_git_dirty() to avoid collision Oct 28, 2014
@mcornella
Copy link
Member

My bad, I didn't cc Robby and so it seemed that this wasn't ready to be merged. I don't know why I missed that, but I understand that it looked like the title was somehow preventing this PR to be merged (which is totally not, and it should have been merged right away). In my defense though, I was triaging at 1AM.

I only wrote that as an addendum to obvious PR, let's get this merged /cc @robbyrussell, but I failed to write that down first. I'm sorry 😅

Now, I understand your frustration, and I just have to say that I agree with you in every way. I've experienced the same delay and neglect, but I'm still doing this for reasons that escape me. I guess I see the huge potential of this project and I have faith in the day when Robby opens up the project or something. FWIW, I don't think he's procrastinating or anything; for instance, I know that he collaborates in an organization for the homeless. I just have to remember that every now and then.

As for supporters of this PR, I thought I saw people asking for the same thing, but I cannot find it now. The closest I got was #2404. But, again, this is not necessary to demonstrate the need to merge this change.

I appreciate the time you took to write what you call "rant" too, and in no way I feel it directed towards me so you don't have to apologize. It's one of the rare instances of explicitly written frustration I've seen in the project, which at least shows how much people care about this.

Thanks and godspeed!


/cc @robbyrussell: good (and urgent) to merge

@michaelorr
Copy link
Contributor Author

@mcornella Thanks for your response and follow-up. I appreciate seeing traction on this.
It's good to keep in mind that for projects like omz, we are all hobbyists.

robbyrussell added a commit that referenced this pull request Nov 6, 2014
Renaming config for parse_git_dirty() to avoid collision
@robbyrussell robbyrussell merged commit 160abc9 into ohmyzsh:master Nov 6, 2014
@robbyrussell
Copy link
Member

@michaelorr Apologies on the latency here! Thanks for your help and contribution!

@mcornella
Copy link
Member

Wonderful news, thank you! ✨

@javache
Copy link

javache commented Nov 6, 2014

Much thanks! 🍰

@dcorb
Copy link

dcorb commented Nov 6, 2014

I've been also thinking why this haven't get merged before (I have had a patched zsh in the meanwhile)
Really glad all worked out and things are now cool again.

Peace! Gracias!

@michaelorr
Copy link
Contributor Author

Thank you @robbyrussell! Much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants