-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Conversation
That doesn't really keep the history/ contribution accurate.
Why not make fork my branch and add test then submit as PR to me. I merge and then it updates the PR I have open. I believe.
…On 15 Apr 2019, 15:46, at 15:46, Marco Rossignoli ***@***.***> wrote:
Replacement of #35809
I moved change on new PR for convenience(few line of code) and added a
test.
If it's not ok (the solution is of @2E0PGS I only added test) please
tell me how I can move commit from another forked branch to mine(so I
don't waste time scrubbing on internet), I neved did it.
Fixes https://github.com/dotnet/corefx/issues/34437
/cc @2E0PGS @danmosemsft @krwq @wtgodbe
You can view, comment on, or merge this pull request online at:
#36880
-- Commit Summary --
* add test
-- File Changes --
M
src/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Linux.cs
(9)
M src/System.Diagnostics.Process/tests/ProcessTestBase.cs (14)
M src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs (47)
-- Patch Links --
https://github.com/dotnet/corefx/pull/36880.patch
https://github.com/dotnet/corefx/pull/36880.diff
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#36880
|
Note that we squash history before committing PR'S to master. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good except for minor comments. Thanks!
So are we just going to ignore that I won't show as a contributor despite my contributions?
Sure the PR mentions my name but GitHub contributors and blame doesn't. Ideally you shouldn't copy and paste that's not making use of the VCS.
Squashing I could of done if required. Then you have one commit from me and one from Marco with a test.
Cherry pick would be better than copy paste. History is preserved.
…On 15 Apr 2019, 17:04, at 17:04, Marco Rossignoli ***@***.***> wrote:
MarcoRossignoli commented on this pull request.
> @@ -58,6 +58,53 @@ public void
GetProcessesByName_RemoteMachineNameUnix_ThrowsPlatformNotSupportedE
Assert.Throws<PlatformNotSupportedException>(() =>
Process.GetProcessesByName(currentProcess.ProcessName, machineName));
}
+ [Fact]
C:\Program Files (x86)\Windows Resource Kits\Tools\sleep.exe on my
machine....I think it's installed also on some CI machine...BTW in that
case will skip.
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#36880 (comment)
|
I agree with @2E0PGS that this should start with his squashed changes and changes on top of that separately - after that we should merge/rebase (not squash) this |
So long as we end up with a single (non merge) commit in master, any variation is fine. |
@2E0PGS feel free to copy paste my code it's not a problem to me at all!Don't worry! |
@MarcoRossignoli you should be able to push your change to the existing PR #35809. @2E0PGS might need to give you rights, give it a try. |
Ok |
@2E0PGS I need permission to write to your branch
|
It may also work to cherry-pick the change by @2E0PGS into your own branch (if you want to play more with Git 😄 ) I do not know which person is considered the "author" when we squash to merge. I'm guessing the author of the first commit. It's fine if authorship works out but as I mentioned, we generally don't pay much attention to that and most folks seem OK with that. |
Added Marco's write perms. push to my new branch https://github.com/2E0PGS/corefx/tree/b/linux-proc-name-char there is a pr for that now. |
@2E0PGS added test, I think you need to login for CLA. |
done. |
Replacement of #35809
I moved changes to new PR for convenience(few line of code) and added a test.
If it's not ok (the solution is of @2E0PGS I only added test) please tell me how I can move commit from another forked branch to mine(so I don't waste time scrubbing on internet), I neved did it.
Fixes https://github.com/dotnet/corefx/issues/34437
/cc @2E0PGS @danmosemsft @krwq @wtgodbe
Output without changes: