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

feat: set node version for astro #5269

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

estephinson
Copy link
Contributor

@estephinson estephinson commented Aug 30, 2023

@estephinson estephinson requested review from a team as code owners August 30, 2023 20:26
nickytonline
nickytonline previously approved these changes Aug 30, 2023
Copy link
Contributor

@nickytonline nickytonline left a comment

Choose a reason for hiding this comment

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

Thanks Ed! 🚢

@@ -27,13 +27,18 @@ export class Astro extends BaseFramework implements Framework {
dark: '/logos/astro/dark.svg',
}

env = {
NODE_VERSION: '18',
Copy link
Contributor

Choose a reason for hiding this comment

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

Mhm I think we should check if a node version is already set in the environment variables. What if a user wants to use node 20?

So we should first check if a node version is set that is larger than 18 and if an environment variable is set.

There are basically 2 ways to set a node version through the UI (should be available through the this.project.getCurrentNodeVersion and this.project.getEnv('NODE_VERSION') if the user set's the env variable we should not set it regardless of if it is greater or not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah thats a great point, just looked at how the other frameworks did it and went with that. I vaguely remember being told the user's will always be respected, but hey

Copy link
Contributor

@lukasholzer lukasholzer left a comment

Choose a reason for hiding this comment

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

@github-actions github-actions bot added the stale label Nov 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants