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

Update development infrastructure #205

Merged
merged 8 commits into from
Apr 11, 2019
Merged

Update development infrastructure #205

merged 8 commits into from
Apr 11, 2019

Conversation

JunTaoLuo
Copy link
Contributor

Addresses #175. Most of the infrastructure is taken from aspnet/AspNetCore.

The get-dotnet.ps1/sh will now install locally to {repoRoot}.dotnet/ and the activate.sh/ps1 will set the appropriate environment variables to use the installed SDK. Edited README to reflect the new workflow.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
JamesNK and others added 3 commits April 9, 2019 22:36
Co-Authored-By: JunTaoLuo <johluo@microsoft.com>
@JunTaoLuo
Copy link
Contributor Author

@JunTaoLuo
Copy link
Contributor Author

Interop test is not required. I think there just needs to be an approval.

@JunTaoLuo JunTaoLuo merged commit 50fbfa0 into grpc:master Apr 11, 2019
@JunTaoLuo JunTaoLuo deleted the johluo/infra branch April 11, 2019 19:26
@jtattermusch
Copy link
Contributor

jtattermusch commented Apr 12, 2019

Looks like this has broken the interop test build on grpc/grpc repository: grpc/grpc#18431
We need to be more careful when updating the way things are built, because grpc/grpc also runs a continuous interop test against master of grpc-dotnet. Basically whenever the script that runs interop tests in this repo is changing, same changes need to be done in grpc/grpc to avoid breakage.
(I would have probably anticipated this problem but this PR was merged without my LGTM)

@jtattermusch
Copy link
Contributor

I might have spoken too soon - I didn't notice grpc/grpc#18714 earlier. But we should have synced more around merging this PR (and subsequently breaking grpc/grpc) anyway.

@JamesNK
Copy link
Member

JamesNK commented Apr 15, 2019

Sorry for creating issues with this PR. We underestimated the impact of changing the build in other places.

I think for now we'll always get your approval before merging, except if the PR has low risk/impact changes such as:

  • A minor refactor or fix
  • Unit test changes
  • Sample or documentation

Does that sound good to you?

@jtattermusch
Copy link
Contributor

Sorry for creating issues with this PR. We underestimated the impact of changing the build in other places.

I think for now we'll always get your approval before merging, except if the PR has low risk/impact changes such as:

  • A minor refactor or fix
  • Unit test changes
  • Sample or documentation

Does that sound good to you?

SGTM.

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.

3 participants