Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Conversation

@A-And
Copy link

@A-And A-And commented Jul 2, 2018

Modifying NETCI definition to stage for Unix version of #18365

@A-And A-And requested a review from BruceForstall July 2, 2018 20:00
@A-And
Copy link
Author

A-And commented Jul 2, 2018

@dotnet-bot test ci please

@A-And A-And force-pushed the FXCIUnixStaging branch from 8f500fb to 6613a06 Compare July 3, 2018 22:18
@A-And A-And force-pushed the FXCIUnixStaging branch from 6613a06 to 34a1eda Compare July 3, 2018 22:38
@A-And
Copy link
Author

A-And commented Jul 3, 2018

@dotnet-bot test ci please

Copy link

@BruceForstall BruceForstall left a comment

Choose a reason for hiding this comment

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

You are adding default-triggered jobs for Ubuntu x64 Checked & Release, and OSX10.12 Checked & Release.

I thought we agreed to only have Checked default triggered jobs. If you want to create triggers for Debug and Release also (but not default triggered), that would be fine.

Also, can you at the same time remove the default trigger for the Windows Release jobs?

@A-And
Copy link
Author

A-And commented Jul 3, 2018

@BruceForstall - Sure thing! I had misunderstood that we'd disable Release configurations entirely.

@A-And
Copy link
Author

A-And commented Jul 3, 2018

@dotnet-bot test ci please

Remove IsJitStressTestScenario assert
@A-And A-And force-pushed the FXCIUnixStaging branch from 5e7fdfd to 8ce4b69 Compare July 4, 2018 22:11
@A-And
Copy link
Author

A-And commented Jul 4, 2018

@dotnet-bot test ci please

@A-And A-And force-pushed the FXCIUnixStaging branch from 40fe2d1 to 6e102d9 Compare July 5, 2018 17:30
@A-And
Copy link
Author

A-And commented Jul 5, 2018

@dotnet-bot test ci please

1 similar comment
@A-And
Copy link
Author

A-And commented Jul 5, 2018

@dotnet-bot test ci please

netci.groovy Outdated
break
case 'corefx_innerloop':
if (os != 'Windows_NT'|| architecture != 'x64') {
if ( (os != 'Windows_NT' && os != 'Ubuntu' && os != 'OSX10.12') || architecture != 'x64') {

Choose a reason for hiding this comment

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

Can you please split this into two conditions:

if (os != 'Windows_NT' && os != 'Ubuntu' &&  os != 'OSX10.12') {
   return false
}
if (architecture != 'x64') {
  return false
}

netci.groovy Outdated
return false
}
if(configuration != 'Release' && configuration != 'Checked') {
if(configuration != 'Checked') {

Choose a reason for hiding this comment

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

I don't think there's any reason to restrict this to just Checked. Seems like we should just delete this condition and allow all of Debug/Checked/Released jobs.

}

else if (scenario == 'corefx_innerloop') {
if (configuration == 'Checked') {

Choose a reason for hiding this comment

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

To allow non-debug triggers for Debug/Release, this (and the equivalent other code, below) should be:

if (configuration == 'Checked') {
  Utilities.addGithubPRTriggerForBranch(job, branch, "${os} ${architecture} ${configuration} CoreFX Tests")
} else {
  Utilities.addGithubPRTriggerForBranch(job, branch, "${os} ${architecture} ${configuration} CoreFX Tests", "(?i).*test\\W+${os}\\W+${architecture}\\W+${configuration}\\W+CoreFX Tests.*")
}

@A-And A-And force-pushed the FXCIUnixStaging branch from 1b9fa54 to dfaf5e4 Compare July 5, 2018 18:14
@A-And A-And force-pushed the FXCIUnixStaging branch from dfaf5e4 to 06246cf Compare July 5, 2018 18:19
@A-And
Copy link
Author

A-And commented Jul 5, 2018

@dotnet-bot test ci please

1 similar comment
@A-And
Copy link
Author

A-And commented Jul 5, 2018

@dotnet-bot test ci please

@A-And A-And merged commit 7baedfd into dotnet:master Jul 9, 2018
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.

2 participants