Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

Comments

Pull please#1

Merged
1 commit merged intodlang:masterfrom
braddr:pull-please
Jan 24, 2011
Merged

Pull please#1
1 commit merged intodlang:masterfrom
braddr:pull-please

Conversation

@braddr
Copy link
Member

@braddr braddr commented Jan 24, 2011

I checked in to my fork the pthread changes to the posix types.d file that Sean and I talked about over email. This is partially to exercise the github pull request mechanism to see how it works. :)

@jmdavis
Copy link
Member

jmdavis commented Jan 24, 2011

Done.

@jmdavis
Copy link
Member

jmdavis commented Jan 24, 2011

That was quite straightforward actually, since github basically gives you all of the necessary instructions.

@jasone
Copy link
Contributor

jasone commented Jan 24, 2011

Somehow I was expecting github to provide a button that would automatically merge the changeset, but instead it provides instructions with a series of git commands. Anyway, this change is merged now.

@braddr
Copy link
Member Author

braddr commented Jan 24, 2011

It's easy, sure. But it also opens exactly an issue I've been concerned about.. areas of responsibility. Changes to druntime need appropriate discussion and some checks and balances before being merged in.

This is something that Sean and I have been discussing in email and it wasn't clear that it was the right thing to do.

@jmdavis
Copy link
Member

jmdavis commented Jan 24, 2011

Well, you wanted someone to merge it in, so I did. It would make sense though to have an appropriate process for deciding whether a pull request gets merged in - especially if patches start coming in that way. Typically, I'd expect a pull request to come from someone who doesn't have write access to the repository. In which case, it becomes primarily a question of how we deal with proposed patches to druntime.

@braddr
Copy link
Member Author

braddr commented Jan 24, 2011

If I'd wanted it checked in without consideration, I could have just pushed it in directly. :)

But, I admit I didn't make it at all clear in my pull request what the status of that code was. Let's take this out of this pull request over to the mailing list for further discussion.

@jmdavis
Copy link
Member

jmdavis commented Jan 24, 2011

Well, then I misunderstood. Sorry.

MartinNowak pushed a commit that referenced this pull request Jul 18, 2012
MartinNowak pushed a commit that referenced this pull request Oct 31, 2013
regression test for Issue 10701
rainers added a commit that referenced this pull request Jun 13, 2014
missing executeArgs for benchmarks
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants