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

Dropped node version back to 8.9.0 while it is still in Maintenance LTS #298

Merged
merged 6 commits into from
Apr 10, 2019

Conversation

bigkraig
Copy link

@bigkraig bigkraig commented Apr 9, 2019

This came up in danger/danger-js#855 where we are adding GitLab support to Danger JS. I didn't see any specific reason to drop support for 8.x which is still supported until Dec 2019.

@bigkraig bigkraig mentioned this pull request Apr 9, 2019
@bigkraig bigkraig changed the title Dropped node version back to 8.9.0 to support more users Dropped node version back to 8.9.0 while it is still in Maintenance LTS Apr 9, 2019
@jetersen
Copy link
Contributor

jetersen commented Apr 9, 2019

Please update Travis to build against 8.9.0 as well.

@bigkraig
Copy link
Author

bigkraig commented Apr 9, 2019

@Casz any idea whats going on with travis? I am not very familiar with how to configure this but I assumed adding another version of node wouldn't create new jobs that also ran the integration tests like that.

…the default test run to not be a full integration test
@bigkraig
Copy link
Author

bigkraig commented Apr 9, 2019

@Casz all set!!

@jdalrymple
Copy link
Owner

I'm going to check and see if we can move the two version tests into their respective stages. Not sure if it will work tbh, but if it doesnt, ill just revert it.

@jetersen
Copy link
Contributor

jetersen commented Apr 9, 2019

@jdalrymple
Copy link
Owner

@Casz That method gets rid of the stages though :(. I found this: travis-ci/travis-ci#8295 ?

Modifying the tests to include two nodejs versions
@jdalrymple
Copy link
Owner

So i added the modifications, but the integration tests seem to fail for 8.9.0. Not sure why exactly :s

@bigkraig
Copy link
Author

bigkraig commented Apr 9, 2019

So i added the modifications, but the integration tests seem to fail for 8.9.0. Not sure why exactly :s

I think we just need to run the docker-compose as part of the 8.9.0 integration test, will know shortly.

@bigkraig
Copy link
Author

bigkraig commented Apr 9, 2019

@Casz @jdalrymple all green! 👍

@jdalrymple
Copy link
Owner

So the before_script part also had to be duplicated ? That's annoying lol. Looks good though! Ill merge and release tonight

Fixing error in alias syntax
@jetersen
Copy link
Contributor

I think you can clean up the travis file even more 😅 Let me try it out.

@jetersen
Copy link
Contributor

Nope 😢

@jdalrymple
Copy link
Owner

Had my hopes up there for a moment @Casz

@jdalrymple jdalrymple merged commit f7fac22 into jdalrymple:next Apr 10, 2019
@jetersen
Copy link
Contributor

So did I 😞

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.

None yet

3 participants