-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Added netci for jenkins #4
Conversation
['Debug', 'Release'].each { config -> | ||
def lowerCaseConfig = config.toLowerCase() | ||
|
||
def newJobName = InternalUtilities.getFullJobName(project, "windows_$lowerCaseConfig", isPR) |
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.
Shouldn't we test on non-windows as well?
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.
Tagging @tannergooding, @jaredpar, @mmitche for review. |
LGTM |
// TODO: For when we actually have unit tests in this repo | ||
// Utilities.addXUnitDotNETResults(myJob, '**/xUnitResults/*.xml') | ||
|
||
Utilities.setMachineAffinity(newJob, 'Windows_NT') |
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.
You need to additionally specify either latest-or-auto-internal
or latest-dev15-internal
(both contain dev12 and dev14, but the latter also contains dev15). For example: Utilities.setMachineAffinity(newJob, 'Windows_NT', 'latest-dev15-internal')
.
This is required for all internal jobs to ensure nothing can be maliciously pulled off a machine by an arbitrary PR made against a public repo.
Overall the changes LGTM, but you need to ensure you explicitly specify an internal machine label before this gets merged. |
|
||
def newJob = job(newJobName) { | ||
steps { | ||
batchFile("build.cmd /$lowerCaseConfig") |
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.
I changed this in my PR last night. This should be
'build.cmd -Configuration $config'
OCD NIT: If you put a line break between your paragraph and start of the numbered list, Github will format it as an actual list. |
def project = GithubProject | ||
def branch = GithubBranchName | ||
|
||
// Generate a PR/nonPR job for debug (test only), which just does testing. |
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.
Fix comment now that only or jobs are generated.
@eerhardt @tannergooding if I can get a final approval after my updates, I'll merge these and submit a pull to dotnet-ci-internal to add this repo to the list. |
LGTM |
Adding Support for publishing using MsDeploy
This adds a netci config for Jenkins. We still need to do the following tasks to get Jenkins fully building PRs and merges: