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

Add AppVeyor #758

Merged
merged 19 commits into from
Feb 4, 2019
Merged

Add AppVeyor #758

merged 19 commits into from
Feb 4, 2019

Conversation

Jean85
Copy link
Collaborator

@Jean85 Jean85 commented Jan 29, 2019

Since #757 didn't work out, this is an attempt to have AppVeyor as a CI for Windows. I've took inspiration from the build that Symfony does: https://github.com/symfony/symfony/blob/master/.appveyor.yml

@Jean85 Jean85 added this to the Release 2.0 milestone Jan 29, 2019
@Jean85 Jean85 self-assigned this Jan 29, 2019
.appveyor.yml Outdated Show resolved Hide resolved
.appveyor.yml Show resolved Hide resolved
@untitaker
Copy link
Member

AppVeyor is running now.

@ste93cry
Copy link
Collaborator

Just asking: why there are two tasks for AppVeyor (continuous-integration/appveyor/branch and continuous-integration/appveyor/pr)? Also can't we use the GitHub apps instead of the old OAuth integrations so that we can use the Checks feature?

@stayallive
Copy link
Collaborator

There are probably 2 builds 1 for the current branch and 1 for the result if the PR was merged (so target branch + pr) I believe.

pr:

git clone -q https://github.com/getsentry/sentry-php.git C:\projects\sentry-php
git fetch -q origin +refs/pull/759/merge:
git checkout -qf FETCH_HEAD

+refs/pull/759/merge is the branch GitHub auto-creates to check if it can be merged.

branch:

git clone -q --branch=fix/error_types-option https://github.com/getsentry/sentry-php.git C:\projects\sentry-php
git checkout -qf 2945301d9c04ef090300dc2f2c6aa26adeea0c15

@ste93cry
Copy link
Collaborator

Mmm, it's fine but now I'm wondering why Travis CI doesn't do the same...

@stayallive
Copy link
Collaborator

Possibly turned off. Looks like Travis only checks the "merged pr" branch (which would be fine for appveyor too I think).

@Jean85
Copy link
Collaborator Author

Jean85 commented Jan 31, 2019

I agree, the build in the branch is superfluous.

@Jean85 Jean85 changed the title [WIP] Add AppVeyor Add AppVeyor Feb 4, 2019
Copy link
Collaborator Author

@Jean85 Jean85 left a comment

Choose a reason for hiding this comment

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

Build is green! No longer WIP, please review and merge.

.appveyor.yml Show resolved Hide resolved
.appveyor.yml Show resolved Hide resolved
src/Options.php Show resolved Hide resolved
Copy link
Collaborator

@stayallive stayallive left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@ste93cry ste93cry merged commit a78a4b1 into master Feb 4, 2019
@ste93cry ste93cry deleted the add-appveyor branch February 4, 2019 23:18
@Jean85 Jean85 mentioned this pull request Feb 5, 2019
@Jean85 Jean85 mentioned this pull request Feb 18, 2019
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.

4 participants