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

Test node in all current versions #19

Merged
merged 1 commit into from Aug 12, 2019

Conversation

wesleytodd
Copy link
Contributor

Node packages should be tested in all current versions by default. We are working on setting some guidelines as mentioned in actions/setup-node#25, so until aliases are supported we would want to keep this list up to date with the node LTS plan: https://nodejs.org/en/about/releases/

As I am not in the beta, I do not have the ability to try these out, but I wanted to make sure we setup people up with the best practice from day one, so figured I would get this in early. I copied from the python example, so think it should work but am happy to make any changes necessary to get this merged.

@damccorm
Copy link

damccorm commented Aug 9, 2019

This generally LGTM. My only big question is should we do this as the only template or as a separate template (and keep the non-matrixed version)? If we do both, which should be the default node template?

I'm pretty non-opinionated on this one, @chrispat might have more thoughts?

@wesleytodd thanks for the quick follow through on this!

@wesleytodd
Copy link
Contributor Author

Hopefully the point of making this the only one is that it is the highly recommended default for packages. If you are also thinking about service testing then I could see having a secondary template, but it seems like OSS packages are the most important for this.

Copy link

@damccorm damccorm left a comment

Choose a reason for hiding this comment

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

Cool, that makes sense to me. Thanks for doing this!

@damccorm damccorm merged commit c8c75ef into actions:master Aug 12, 2019
@chrispat
Copy link
Member

@wesleytodd and @damccorm would it make more sense here to have LTS and LATEST as the options in this matrix given the goals?

@damccorm
Copy link

damccorm commented Aug 12, 2019

Maybe eventually, we still need to add those options though. actions/setup-node#26

@damccorm
Copy link

That should be pretty quick to implement when the time comes, just want to make sure we're getting the options right though before we do the work and people start to rely on them.

@wesleytodd
Copy link
Contributor Author

Is there anyway one of you could get me into the beta so I can start using these? We talked in the last ExpressJS TC meeting about getting more automation running, and now that you announced this I think we would prefer to keep that all on github.

@jeremyepling
Copy link
Contributor

@wesleytodd have you signed up expressjs for the beta? https://github.com/features/actions/signup/?account=expressjs

After that I can get you in.

@wesleytodd
Copy link
Contributor Author

I was not aware you could register on behalf of an organization, so I had just done it as myself :)

Thanks!

@chrispat
Copy link
Member

you should be enabled now

@damccorm damccorm mentioned this pull request Aug 14, 2019
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

4 participants