Skip to content

Conversation

@nschonni
Copy link
Contributor

The hook no longer appears to be used and doesn't build correctly for forked repos either

@@ -1,29 +0,0 @@
language: csharp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nschonni this file is referenced from the following section:
https://docs.microsoft.com/en-us/dotnet/core/tools/using-ci-with-cli#travis-ci

as the example of the .travis.yml file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I'll see about inlining the content there as-is. Currently this just causes people forking the repo to get build failure emails when they create branches or PRs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's already pointing to the Travis docs, so that should be good now that they have sections on .Net Core

@nschonni nschonni requested a review from mairaw as a code owner October 14, 2018 21:05
@mairaw
Copy link
Contributor

mairaw commented Oct 16, 2018

@KathleenDollard @livarcocc @dsplaisted can you guys review this PR? The original file was specific to .NET Core but has been severely outdated. Maybe we can update the travis (and the appveyor files from the previous PR) and link to their docs for extra info?

@richlander
Copy link
Member

If we don't have a sponsor for travis integration, we should delete the sample. I agree that it is very out of date.

Stating the obvious, we should probably ensure we have good docs for DevOps integration as a higher priority.

CI samples should not be in this repo, but in the samples (or other) repo. Right?

@BillWagner
Copy link
Member

closing based on the above conversation.

@BillWagner BillWagner closed this Dec 17, 2018
@nschonni
Copy link
Contributor Author

@BillWagner the above conversation was to delete if it's not going to be maintained. Are you going to redo this PR to remove the file?

@BillWagner
Copy link
Member

@nschonni Sorry about that. I lost the context over time. Looking at it more closely, it looks like you've done exactly what @richlander recommended.

Should I reopen and merge this?

@mairaw
Copy link
Contributor

mairaw commented Dec 17, 2018

I think so @BillWagner. I think it would also be good to sync with @erickson-doug's team on the DevOps docs related to .NET scenarios.

@mairaw mairaw reopened this Dec 17, 2018
@mairaw
Copy link
Contributor

mairaw commented Dec 17, 2018

Resolved merge conflicts and did a small fix. I'll merge when build is green.

@mairaw mairaw merged commit b174f53 into dotnet:master Dec 17, 2018
@nschonni nschonni deleted the patch-17 branch December 17, 2018 22:15
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.

5 participants