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 workflows, merge repository kuzzle-containers #2502

Merged
merged 24 commits into from
Nov 29, 2023
Merged

Conversation

rolljee
Copy link
Contributor

@rolljee rolljee commented Nov 28, 2023

What does this PR do ?

  • This PR update workflows files to be more readable and up to date. This will prepare the work for NodeJS 20
  • Merging Kuzzle-containers repository here, because it make more sense now that kuzzle build and push all image that he needs.

@rolljee rolljee self-assigned this Nov 28, 2023
@rolljee rolljee linked an issue Nov 28, 2023 that may be closed by this pull request
Comment on lines 10 to 12
NODE_LTS_MAINTENANCE_VERSION: "16"
NODE_LTS_ACTIVE_VERSION: "18"
NODE_LTS_CURRENT_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.

Why not use a matrix for doing the same thing?

@@ -20,7 +20,7 @@ jobs:
steps:
- id: set-matrix
run: |
echo "::set-output name=matrix::{\"node-version\": [\"$NODE_LTS_MAINTENANCE_VERSION\", \"$NODE_LTS_ACTIVE_VERSION\"]}"
echo "matrix={\"node-version\": [\"$NODE_LTS_MAINTENANCE_VERSION\", \"$NODE_LTS_ACTIVE_VERSION\"]}" >> $GITHUB_OUTPUT
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to set up the matrix of Nodejs versions normally (with an array of numbers) instead of constructing it like this? This seems rather "hacky".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I tried a bunch of thing, seems that this is the way to go if we don't want to repeat the matrix over and over for each jobs

Comment on lines 26 to 27

WORKDIR /var/app
Copy link
Contributor

Choose a reason for hiding this comment

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

The working directory was already set above.

Suggested change
WORKDIR /var/app

@rolljee rolljee mentioned this pull request Nov 29, 2023
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 6 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.7% 0.7% Duplication

@rolljee
Copy link
Contributor Author

rolljee commented Nov 29, 2023

We are going to squash this one 🥶

@rolljee rolljee merged commit 4023314 into 2-dev Nov 29, 2023
26 checks passed
@rolljee rolljee deleted the feat/containers branch November 29, 2023 16:46
@rolljee rolljee mentioned this pull request Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade workflows files, and merge kuzzle containers repository
3 participants