Skip to content

feature: ci with multiple targets #35

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 2 commits into from
Oct 13, 2021

Conversation

jankapunkt
Copy link
Member

This implements #34 in a way that our CI always tests for multiple node versions. Also removed the old travis file.

@jankapunkt jankapunkt marked this pull request as ready for review October 13, 2021 07:15
@jankapunkt jankapunkt added the enhancement ✨ New feature or request label Oct 13, 2021
@jankapunkt jankapunkt self-assigned this Oct 13, 2021
@jankapunkt jankapunkt linked an issue Oct 13, 2021 that may be closed by this pull request
@jankapunkt
Copy link
Member Author

This was referenced Oct 13, 2021
@HappyZombies
Copy link
Member

@jankapunkt you need to target development branch! Not master ;)

Also, per this conversation about the linter, we realized that ESLint actually runs a specific version of Node 12 #30 (comment)

When we add linting to the pipeline, this will be an issue no doubt. So maybe there's a way to target that specific node version? I guess it doesn't really matter, since it's just a dev dependency for ESLint.

@jankapunkt jankapunkt changed the base branch from master to development October 13, 2021 14:01
@jankapunkt
Copy link
Member Author

Hey @HappyZombies my bad, changed.

Regarding eslint it runs fine on all three node versions in the ci so I think this should not be a problem, right?

@HappyZombies
Copy link
Member

HappyZombies commented Oct 13, 2021

@jankapunkt I think you are right, I believe the CI uses whatever latest LTS is for Node 12, which is supported for that specific Node version.

Thanks, I'll do one final check, then approve and merge.

Copy link
Member

@HappyZombies HappyZombies left a comment

Choose a reason for hiding this comment

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

@jankapunkt quick question on the npm run test being removed.

@HappyZombies HappyZombies merged commit 81aac3a into development Oct 13, 2021
@jwerre jwerre deleted the feature-ci-multiple-targets branch November 21, 2021 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Supporting Node12 and above
2 participants