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.

Sign up for GitHub

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

Jump to bottom

Set up AppVeyor to run tests on Windows #6238 #6321

Merged
merged 7 commits into from Jan 31, 2017
Merged

Set up AppVeyor to run tests on Windows #6238 #6321

merged 7 commits into from Jan 31, 2017

Conversation

wkurniawan07
Copy link
Member

Fixes #6238. The third commit also fixes #6179 and fixes #6009.

I'm not sure from which branch the badge pulls the status from, although this is just a minor edit if needed.

Also, I do not include the static analysis tools here (let Travis do the job) but easy to include if needed.

@wkurniawan07
Copy link
Member Author

Seems like there is another unstable test that I haven't remedied :/
But do feel free to review what's already here.

damithc
damithc approved these changes Jan 11, 2017
View reviewed changes
Copy link
Contributor

@damithc damithc left a comment

Choose a reason for hiding this comment

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

Looks fine so far, as long as the disabled test is not failing due to an OS-specific reason.

@wkurniawan07
Copy link
Member Author

Looks reasonably stable now. @damithc can review again.

@wkurniawan07 wkurniawan07 added the s.ToReview The PR is waiting for review(s) label Jan 13, 2017
@damithc damithc added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging and removed s.ToReview The PR is waiting for review(s) labels Jan 14, 2017
@wkurniawan07
Copy link
Member Author

Seems like we're not ready for AV in PRs. There are still some unstable tests coming from time to time (more unstable than they are in Travis), and the velocity we sustain for the past weeks will make AV a bottleneck.
Perhaps it's best to keep AV for master and release only for the time being. That way we still have a certificate that "everything works in Windows".

@damithc
Copy link
Contributor

damithc commented Jan 31, 2017

Perhaps it's best to keep AV for master and release only for the time being. That way we still have a certificate that "everything works in Windows".

Sure, let's do that.

@wkurniawan07 wkurniawan07 merged commit dccb667 into master Jan 31, 2017
@wkurniawan07 wkurniawan07 deleted the 6238-appveyor branch January 31, 2017 06:32
@wkurniawan07 wkurniawan07 mentioned this pull request Jan 31, 2017
Closed
@wkurniawan07 wkurniawan07 added this to the V5.95 milestone Feb 1, 2017
@wkurniawan07 wkurniawan07 modified the milestones: V5.95, V5.95rc Feb 11, 2017
karthikaacharya pushed a commit to karthikaacharya/teammates that referenced this pull request Feb 23, 2017
@wkurniawan07 @karthikaacharya
[TEAMMATES#6238] Set up AppVeyor to run tests on Windows (TEAMMATES#6321
2c6ff5c 
)
@whipermr5 whipermr5 mentioned this pull request Mar 22, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@damithc damithc damithc approved these changes

Assignees
No one assigned
Labels
s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
None yet
Milestone
V5.95
2 participants
@wkurniawan07 @damithc