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

Make AppVeyor check PRs (was: Please add CI build badges to the README) #330

Closed
sschuberth opened this issue Oct 12, 2016 · 15 comments
Closed

Comments

@sschuberth
Copy link
Collaborator

Could we please get build badges as part of the README, for both TravisCI and AppVeyor, and each for the master and develop branches? That helps to quickly see the health status, and to access the latest build artifacts for testing.

@pombredanne
Copy link
Member

@sschuberth Build badges have always been there as long as I recall in the README. See
https://github.com/nexB/scancode-toolkit#build-and-tests-status
There is a table with Windows, Mac and Linux and both develop and master branches (and if you look at the raw text I recall this was a bit involved to get right ;) )

May be they should be instead at the top of the readme?

@sschuberth
Copy link
Collaborator Author

Doh, sorry for that. I was so used from other projects to see these at the top that I simply did not scroll further down. Feel free to close this unchanged, or to move them to the top of the README as you prefer.

But on a related note, I don't see Appveyor builds being enabled for PRs. Is that something you could enable?

@pombredanne
Copy link
Member

OK, so let's move these to the top.
On the appveyor side, I think things are enabled alright as per this: http://help.appveyor.com/discussions/questions/203-auto-run-tests-on-pull-requests and I checked the repo webhooks config whcih is correct. Do you have seen a problem?

@sschuberth
Copy link
Collaborator Author

Well, looking at e.g. #321 and clicking on "Show all checks" I would have expect to see AppVeyor in there, but I only see TravisCI ...

@pombredanne
Copy link
Member

Agreed, I would have loved to see this there too. But the problem is that Appveyor does not offer a "Github integration" but instead only a webhook. Hence the builds do not show up in a GH PR check.
To compound the issue, Appeyor does not have IRC notifications so I cannot even push build notifications to #aboutcode on freenode.net like I push for Travis.

I will not complain: this is a free service but unfortunately as with many things in MSFT-centric development environments, the appveyor config is often arcane and circumvoluted

@pombredanne
Copy link
Member

As a side note, #321 may need some work and in particular the base url to link to a lincense may need be moved to a config option/config files. As it may also breaks some other features that require Jinja (for custom HTML templates, even though this feature may go away).

pombredanne added a commit that referenced this issue Oct 12, 2016
Thank you to @sschuberth for reporting this

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@sschuberth
Copy link
Collaborator Author

But the problem is that Appveyor does not offer a "Github integration" but instead only a webhook. Hence the builds do not show up in a GH PR check.

That's not correct :-) I recently helped to enable AppVeyor CI checks for PRs for the Git LFS project, see e.g. git-lfs/git-lfs#1574.

Appeyor does not have IRC notifications

At least it's on their list: appveyor/ci#88.

@pombredanne
Copy link
Member

hum, I cannot see a way to enable an Appveyor integration though.
Appveyor is configured to have access, but never offered as an integration. And never shows up in the list of commit/pr checks for any branch....
Do you have some insight with how this is setup on the git-lfs GH repo settings?

@pombredanne
Copy link
Member

ATM the way appveyor is configured is with a plain webhook added to the GH repo and setup to receive push and pr events

@sschuberth
Copy link
Collaborator Author

Not sure what went wrong for you. I usually go to https://ci.appveyor.com/projects/new, select the "GitHub" tab, and add the project I want to have built. That makes PRs for me build out of the box.

@pombredanne
Copy link
Member

OK, let me try to delete the integrations and webhook and recreate that from scratch

@sschuberth sschuberth changed the title Please add CI build badges to the README Make AppVeyor check PRs (was: Please add CI build badges to the README) Oct 13, 2016
@pombredanne
Copy link
Member

I deleted and recreated the Appveyor projects and it looks like this fixed the problem.

pombredanne added a commit that referenced this issue Oct 21, 2016
 * based on @crwood https://github.com/gridsync/gridsync irc-notify.py
 * GPL-licensed
 * also updated the appveyor config to send IRC notifications

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne
Copy link
Member

And added a script forked from @crwood for IRC notifications. This covers appveyor/ci#88 nicely

@pombredanne
Copy link
Member

And you can see the first build working here: https://ci.appveyor.com/project/nexB/scancode-toolkit/build/1
And it was notified on freenode #aboutcode channel alright. Success! I will try a PR next.
Now the only dark side is that we lost the build history.

@pombredanne
Copy link
Member

@sschuberth as you can in #334 we now both IRC notification and PR properly showing Appveyor as a check.
Thanks for the report!
Closing now

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

No branches or pull requests

2 participants