Skip to content

Fix bug with options not being copied correctly #794

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

Merged
merged 3 commits into from
Jan 2, 2015

Conversation

mgreter
Copy link
Contributor

@mgreter mgreter commented Dec 31, 2014

This is one part of a fix for sass/sassc#88
Sassc related PR: sass/sassc#89
Also adds more assertions for invalid context options!

@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) when pulling 75a1147 on mgreter:fix/copy-context-options into fa5d68d on sass:master.

@xzyfer xzyfer mentioned this pull request Jan 2, 2015
12 tasks
Fixes sass/sassc#88
Add more assertions for invalid context options
@mgreter mgreter force-pushed the fix/copy-context-options branch from 75a1147 to fa22282 Compare January 2, 2015 00:17
@mgreter
Copy link
Contributor Author

mgreter commented Jan 2, 2015

Appveyor seems to cache the sassc commit it uses somehow when I just ammend the message:

#appveyor mingw build
sassc: 3.0.2-7-gdcd5
libsass: 3.1.0-beta.2-8-g85af
#travis-ci builds
sassc: 3.0.2-16-gfbdc
libsass: 3.1.0-beta.2-8-g85af

@mgreter
Copy link
Contributor Author

mgreter commented Jan 2, 2015

OK, facepalm to myself. Appveyor.yml still contained the url to my local sassc branch ...

@am11
Copy link
Contributor

am11 commented Jan 2, 2015

@mgreter, if you are authorized, you would see a Stop Build (or Skip Build) button on Appveyor. So you can stop these for example:

https://ci.appveyor.com/project/hcatlin/libsass/build/1.0.50
and
https://ci.appveyor.com/project/hcatlin/libsass/build/1.0.51

@mgreter
Copy link
Contributor Author

mgreter commented Jan 2, 2015

Nope, no permission to do that, already checked 😉 Also @hcatlin probably needs to delete his "hcatlin" repo on appveyor so the now setup official one kicks in (hopefully)!

@am11
Copy link
Contributor

am11 commented Jan 2, 2015

Oh great!

On a separate note @xzyfer and I were just chatting here https://gitter.im/sass/~chat# about something which you might be able to help us with! Whenever you have time, you can see. The chat logs don't expire on gitter. 😃

@xzyfer
Copy link
Contributor

xzyfer commented Jan 2, 2015

@mgreter you can rebase out your commits and rebase of master instead to get those commits

@xzyfer
Copy link
Contributor

xzyfer commented Jan 2, 2015

This appears to be fixed in my tests 👍

@am11
Copy link
Contributor

am11 commented Jan 2, 2015

@mgreter, apparently we have access to start/stop here: https://ci.appveyor.com/project/sass/libsass/build/1.0.2. I have started the build! 👌

@xzyfer xzyfer added this to the 3.1 milestone Jan 2, 2015
@am11
Copy link
Contributor

am11 commented Jan 2, 2015

Apparently, we just need to updated the web hook > Payload URL in GH repo settings. Something like this:

payload url

@xzyfer
Copy link
Contributor

xzyfer commented Jan 2, 2015

I have access to change the webhook. Do you know the correct webhook URL? I can't find anything in the appveyor ui.

@am11
Copy link
Contributor

am11 commented Jan 2, 2015

@xzyfer https://ci.appveyor.com/api/github/webhook?id=s88o0u5qra2ng4vy
It corresponds to sass/libsass.

@xzyfer
Copy link
Contributor

xzyfer commented Jan 2, 2015

OK updated!

@xzyfer
Copy link
Contributor

xzyfer commented Jan 2, 2015

Wow 1hr AppVeyor builds is a serious problem 👎

@am11
Copy link
Contributor

am11 commented Jan 2, 2015

LOL yeah. Some loop in there. I think unless there is a new commit pushed (or rebased/amend the existing one), it will not pick up sass/libsass ?

@xzyfer
Copy link
Contributor

xzyfer commented Jan 2, 2015

It picked up the build fine. It actually just takes an hour to run all the tests.

screen shot 2015-01-02 at 12 53 46 pm

@xzyfer
Copy link
Contributor

xzyfer commented Jan 2, 2015

Oh wow I just realised that's not even this branch 😭

xzyfer added a commit that referenced this pull request Jan 2, 2015
Fix bug with options not being copied correctly
@xzyfer xzyfer merged commit 31521ef into sass:master Jan 2, 2015
@xzyfer
Copy link
Contributor

xzyfer commented Jan 4, 2015

@am11 it appear altering that webhook url has broken our AppVeyor integration. Where did you find the that webhook URL in the AppVeyor interface?

@am11
Copy link
Contributor

am11 commented Jan 4, 2015

@xzyfer, seems like it is building the latest release: https://ci.appveyor.com/project/sass/libsass/.
The Webhook URL is located on the project settings main page: https://ci.appveyor.com/project/sass/libsass/settings.

@xzyfer
Copy link
Contributor

xzyfer commented Jan 4, 2015

Hmm I don't appear to have access.

It's building the releases but not pinging back to github and showing up in PRs.

@am11
Copy link
Contributor

am11 commented Jan 4, 2015

I think if you are an admin in Sass, then it should allow you to access and modify settings.
Meanwhile, here is how I have it configured in one of my Repo on GitHub:
appveyor-webhook2

You can perhaps try deleting the entry and creating a new one with https://ci.appveyor.com/api/github/webhook?id=s88o0u5qra2ng4vy.

@HamptonMakes
Copy link
Member

I just checked and the settings for libsass webhooks seem to match this. Is there anything you need me to do as an admin?

@xzyfer
Copy link
Contributor

xzyfer commented Jan 4, 2015

@hcatlin can you please add myself and mgreter to the github sass team so
we can administer AppVeyor?
On 5 Jan 2015 06:39, "Hampton Catlin" [email protected] wrote:

I just checked and the settings for libsass webhooks seem to match this.
Is there anything you need me to do as an admin?


Reply to this email directly or view it on GitHub
#794 (comment).

@xzyfer
Copy link
Contributor

xzyfer commented Jan 4, 2015

From gitter chat with @am11

@xzyfer, there is permission tab, which says administrators have full access (and no way to invite anyone). I think it is inheriting permissions from GitHub. Which makes sense because I can't find you guys here https://github.com./orgs/sass/people
@andrew @hcatlin, guys please add @xzyfer and @mgreter to the Sass, the same way you added us; so their membership is visible via GH API.

@andrew
Copy link

andrew commented Jan 4, 2015

@xzyfer I added you and @mgreter to this team: https://github.com./orgs/sass/teams/node-sass-owners, hopefully that will sort it, you should get an email about it.

@xzyfer
Copy link
Contributor

xzyfer commented Jan 4, 2015

Aha, success! Thanks @andrew it appear to have access now.

@am11
Copy link
Contributor

am11 commented Jan 4, 2015

👍 🎉

@mgreter mgreter deleted the fix/copy-context-options branch April 6, 2015 17:14
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.

6 participants