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

do not merge - just testing something #12763

Closed
wants to merge 2 commits into from
Closed

Conversation

pieh
Copy link
Contributor

@pieh pieh commented Mar 22, 2019

this is just for testing purposes - I will close this later

@pieh pieh requested a review from a team as a code owner March 22, 2019 16:26
@@ -50,6 +50,7 @@ aliases:
test_template: &test_template
steps:
- checkout
- run: node -v
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😬 hopefully the executors are working like we'd expect! Good idea.

@pieh
Copy link
Contributor Author

pieh commented Mar 22, 2019

This is just to test something that came up in #12719 - because according to info there - it should fail for node6 (as some module is node8+ only), but it passed there :S

@pieh
Copy link
Contributor Author

pieh commented Mar 22, 2019

It reports expected node version - so not sure why test in PR linked above doesn't fail in some ways if it needs node 8+ stuff - hmm, let's check something more

@DSchau
Copy link
Contributor

DSchau commented Mar 22, 2019

@pieh that package could be following our approach, haha. e.g. "soft deprecating" but still supporting?

@pieh
Copy link
Contributor Author

pieh commented Mar 22, 2019

That's also possible. Maybe just parts of the package that we use might not require 8+?

@pieh
Copy link
Contributor Author

pieh commented Mar 22, 2019

Ok, so tests that is designed to fail in node 6 and pass in node 8+ works - so our tests doesn't seem to be wonky after all

@pieh pieh closed this Mar 22, 2019
@DSchau DSchau deleted the circle-ci-test-node-v branch March 22, 2019 16:51
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