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 GitHub Enterprise Support #56

Merged
merged 1 commit into from
Dec 7, 2015
Merged

Conversation

apiv
Copy link

@apiv apiv commented Nov 19, 2015

No octonode yet, wanted to get the fixes in first, so that those of us on enterprise can fully use Strider-CD.

  • Tested with a github enterprise server, working
  • Tested with github.com, working

@knownasilya
Copy link
Member

Think it would be possible to write some tests? Or will it be too difficult?

@apiv
Copy link
Author

apiv commented Nov 19, 2015

I was having trouble coming up with tests to write, as I don't think there is an 'example' github enterprise server we can try to connect to. The only thing I can think of would be to ensure that the environment variables set the appConfig object in ./webapp.js.

Any ideas?

Also, it looks like my checks failed on travis, but they are passing locally. Should I try re-running?

@knownasilya
Copy link
Member

Was thinking of setting the options, and then passing in mock data to see if it still works.

@apiv
Copy link
Author

apiv commented Nov 19, 2015

I'll give that a shot.

@apiv
Copy link
Author

apiv commented Dec 7, 2015

I haven't been able to find a way to test against different set of environment variables, short of doing some hacky things like manually invalidating the require.cache...

It may be too difficult to test around this, but the existing specs do protect the existing functionality.

knownasilya pushed a commit that referenced this pull request Dec 7, 2015
Add GitHub Enterprise Support
@knownasilya knownasilya merged commit 9bbae44 into Strider-CD:master Dec 7, 2015
@knownasilya
Copy link
Member

I'll try to release a new version tomorrow.

@apiv
Copy link
Author

apiv commented Dec 7, 2015

Great, thanks!

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.

2 participants