-
Notifications
You must be signed in to change notification settings - Fork 464
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
Fix some minor build stuff #805
Conversation
Autotools scripts are put in script, not the top directory, so update paths for that.
This should avoid inconsistent errors like the one that showed up in PR sass#800.
Hmm, no AppVeyor? That's really what I was interested in... |
For some reason, Appveyor is not picking up the webhook. @xzyfer updated the webhook URL in settings. @FeodorFitsner, can you please tell us besides updating the webhook URL, what else do we need to change to hear back from https://ci.appveyor.com/project/sass/libsass? |
-Feodor On Mon, Jan 5, 2015 at 4:16 PM, Adeel Mujahid notifications@github.com
|
Thanks @FeodorFitsner! @xzyfer, @mgreter, can you please confirm what @FeodorFitsner's has pointed out? You can find those settings under Appveyor Web Hook settings page on GitHub: (something like https://github.com/sass/libsass/settings/hooks/ It looks like: |
I spent about an hour looking into this. Our setup is exactly the same as node-sass. AppVeyor is pickup the builds, it's just not update the build status. I think issue may be on GH's end... maybe? i dunno |
FWIW @QuLogic this appears to have passed in AppVeyor https://ci.appveyor.com/project/sass/libsass/build/1.0.57 |
Thanks @xzyfer! |
FYI: I have activated "status" recently in the hope it might solve this ... beside that I have no idea. @xzyfer do you have admin access at appveyor for that repository? Maybe we need some setting there? |
Could you paste a screenshot of webhook settings from GitHub? -Feodor On Mon, Jan 5, 2015 at 5:13 PM, Marcel Greter notifications@github.com
|
@mgreter, you and @xzyfer both are admins. :) @FeodorFitsner, its right here: #805 (comment) |
FYI: I confirmed our settings are the same as node-sass in both GH and AppVeyor yesterday. |
@mgreter you'll need to log out of AppVeyor and re-login then select |
OK, and what's correspondent project at AppVeyor? -Feodor On Mon, Jan 5, 2015 at 5:21 PM, Michael Mifsud notifications@github.com
|
I see queued builds: https://ci.appveyor.com/project/sass/libsass/history Seems to be OK? |
@FeodorFitsner The issue is that "CI reports" for PRs are not included. As you can see right below this comment, only Travis-CI reports it's status to the PR: "The Travis CI build passed". Before it read something like "2 out of 2 ..."! |
Will take a look. -Feodor On Mon, Jan 5, 2015 at 6:24 PM, Marcel Greter notifications@github.com
|
Since @xzyfer mentioned that this passed on AppVeyor, I guess we can merge this, the missing CI results notwithstanding. |
Following appveyor/ci#157, I had a brief email conversation with @FeodorFitsner and GitHub staff, I have found the solution for AppVeyor hook problem. Currently, if the owner logon to AppVeyor, select Sass account and browse to https://ci.appveyor.com/account, they will see my account as GitHub authorization. It was set to Andrew's account, I revoked and re-authorize with my account to test whether it keeps the history, which it does. The issue is neither Andrew nor me are owners of LibSass, but only node-sass. So the solution is simple; either @xzyfer, @mgreter or anyone with Owner role for both node-sass and libsass can logon to https://ci.appveyor.com/account with GitHub creds, select Sass account and click the revoke button under GitHub account then click authorize. This will hook both repos. |
Thanks for the hint, seems like that did it! #925 |
🎉 |
In a6b58f4, the aux directory for autotools stuff was changed to
script
, but some things (like ignore files) were not updated to this new path.Secondly, make undefined symbols an error, since it's necessary for Windows anyway. This should make inconsistent failures like in #800 stop occurring (because all builds would fail). I'm not sure if this argument is supported everywhere, so I guess we'll see what happens with the CI...