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

Update node 20 #447

Merged
merged 2 commits into from
Jul 3, 2023
Merged

Update node 20 #447

merged 2 commits into from
Jul 3, 2023

Conversation

difernandez
Copy link
Contributor

  • Se sube a la versión 20 de node
  • Se instala manualmente node y yarn en vez de que salgan de la imagen de ruby, sacando así la versión del .node-version

@difernandez difernandez requested a review from gmq June 30, 2023 21:47
@@ -32,6 +32,14 @@ commands:
gem install bundler:2.1.4
bundle _2.1.4_ install

- run:
name: Install node and yarn
Copy link
Contributor

Choose a reason for hiding this comment

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

No es más lento hacerlo aparte? Aunque potassium de por si es lento de testear asi que no creo que haga mucha diferencia al final

Copy link
Contributor Author

Choose a reason for hiding this comment

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

muy mínimo (y la imagen fue un pelín más rápido igual)
image

@@ -6,7 +6,7 @@ module Potassium
RUBOCOP_RSPEC_VERSION = "~> 2.2"
POSTGRES_VERSION = "11.3"
MYSQL_VERSION = "5.7"
NODE_VERSION = "14"
NODE_VERSION = "20"
Copy link
Contributor

Choose a reason for hiding this comment

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

Se podrá leer de .node-version en vez de definirlo dos veces?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

onda hacer File.open("../../.node-version").read.strip acá?

Copy link
Contributor

Choose a reason for hiding this comment

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

Si, considerando que siempre usamos nodenv-aliases asi que no habría peligro que fuera incompatible con la url usada 🤔

Copy link
Contributor Author

@difernandez difernandez Jul 3, 2023

Choose a reason for hiding this comment

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

No estoy pudiendo hacerlo 🤔 usando ../../.node-version me tira error antes de correr los tests. Usando .node-version corre los tests, pero estos fallan. Traté de usar Rails.root, y no reconoce la constante Rails en esta parte

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Voy a mergear así no más mientras

@difernandez difernandez merged commit 00caf0f into master Jul 3, 2023
@difernandez difernandez deleted the update-node-20 branch July 3, 2023 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants