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

fix: yarn berry + fetch + docker opti + sec #1012

Merged
merged 34 commits into from
Sep 26, 2023
Merged

Conversation

devthejo
Copy link
Member

@devthejo devthejo commented Sep 5, 2023

No description provided.

@devthejo devthejo temporarily deployed to review-auto September 11, 2023 14:37 — with GitHub Actions Inactive
@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@devthejo devthejo marked this pull request as ready for review September 11, 2023 15:09
@devthejo devthejo requested a review from a team as a code owner September 11, 2023 15:09
Copy link
Member

@maxgfr maxgfr left a comment

Choose a reason for hiding this comment

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

top ! avant de merger, il faut qu'on valide avec la team et les points que j'ai noté dans la review

merci beaucoup pour le travail jo 🙏

targets/export-elasticsearch/package.json Show resolved Hide resolved
targets/export-elasticsearch/Dockerfile Show resolved Hide resolved
targets/alert-cli/Dockerfile Outdated Show resolved Hide resolved
targets/frontend/Dockerfile Outdated Show resolved Hide resolved
targets/ingester/Dockerfile Outdated Show resolved Hide resolved
Copy link
Member

@maxgfr maxgfr left a comment

Choose a reason for hiding this comment

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

je l'a mets en request changes

@devthejo devthejo temporarily deployed to review-auto September 25, 2023 18:19 — with GitHub Actions Inactive
@devthejo devthejo temporarily deployed to review-auto September 25, 2023 21:09 — with GitHub Actions Inactive
@devthejo devthejo temporarily deployed to review-auto September 25, 2023 21:38 — with GitHub Actions Inactive
@devthejo devthejo temporarily deployed to review-auto September 25, 2023 22:12 — with GitHub Actions Inactive
@maxgfr maxgfr self-requested a review September 26, 2023 12:23
Copy link
Member

@maxgfr maxgfr left a comment

Choose a reason for hiding this comment

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

Merci pour la PR et les explications tu gères 🙏

@@ -51,13 +51,13 @@
"start": "node --enable-source-maps --unhandled-rejections=strict ./dist/index.js",
"start:debug": "rm -rf data/* && ncc build ./src/index.ts -o ./dist -e nodegit -s && HASURA_GRAPHQL_ENDPOINT=http://localhost:8080/v1/graphql HASURA_GRAPHQL_ADMIN_SECRET=admin1 node --enable-source-maps --unhandled-rejections=strict ./dist/index.js",
"clean": "rm -rf data/*",
"lint": "eslint \"./src/**/*.{js,ts}\"",
"lint": "npx eslint \"./src/**/*.{js,ts}\"",
Copy link
Member

Choose a reason for hiding this comment

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

il faudrait qu'on fix une version sur les dev dependencies. je suis pas fan de passer par npx

Copy link
Member Author

Choose a reason for hiding this comment

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

si eslint est installé dans le node_module du dossier courant ou parent, npx va utiliser la version installée, ce qui est le cas ici, du coup aucun risque que ça prenne la dernière version ;-)
du coup c'est celle là qui sera utilisée:

"eslint": "^8.41.0",

@@ -54,7 +56,7 @@
"clean": "rm -rf data/*",
"cli:dev": "HASURA_GRAPHQL_ENDPOINT=http://localhost:8080/v1/graphql HASURA_GRAPHQL_ADMIN_SECRET=admin1 ncc run -s src/cli.ts",
"cli:prod": "HASURA_GRAPHQL_ENDPOINT=http://localhost:8080/v1/graphql HASURA_GRAPHQL_ADMIN_SECRET=admin1 yarn start",
"lint": "eslint \"./src/**/*.{js,ts}\"",
"lint": "npx eslint \"./src/**/*.{js,ts}\"",
Copy link
Member

Choose a reason for hiding this comment

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

là aussi il faudrait qu'on set la version de eslint

Copy link
Member Author

Choose a reason for hiding this comment

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

si eslint est installé dans le node_module du dossier courant ou parent, npx va utiliser la version installée, ce qui est le cas ici, du coup aucun risque que ça prenne la dernière version ;-)

Copy link
Member Author

Choose a reason for hiding this comment

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

du coup c'est celle là qui sera utilisée:

"eslint": "^8.41.0",

@devthejo devthejo temporarily deployed to review-auto September 26, 2023 12:58 — with GitHub Actions Inactive
Copy link
Contributor

@m-maillot m-maillot left a comment

Choose a reason for hiding this comment

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

Top ! Merci 👍

@sonarcloud
Copy link

sonarcloud bot commented Sep 26, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

No Coverage information No Coverage information
0.0% 0.0% Duplication

@github-actions
Copy link

🎉 Deployment for commit cd0ea8f :

Ingresses
Docker images
  • 📦 docker pull harbor.fabrique.social.gouv.fr/cdtn/cdtn-admin/contributions:sha-cd0ea8fe164f232c7a5038e2fb227168155dae4e
  • 📦 docker pull harbor.fabrique.social.gouv.fr/cdtn/cdtn-admin/export:sha-cd0ea8fe164f232c7a5038e2fb227168155dae4e
  • 📦 docker pull harbor.fabrique.social.gouv.fr/cdtn/cdtn-admin/frontend:sha-cd0ea8fe164f232c7a5038e2fb227168155dae4e
  • 📦 docker pull harbor.fabrique.social.gouv.fr/cdtn/cdtn-admin/hasura:sha-cd0ea8fe164f232c7a5038e2fb227168155dae4e
Debug

@devthejo devthejo merged commit 81bce48 into master Sep 26, 2023
32 checks passed
@devthejo devthejo deleted the fix/yarn-berry-and-fetch branch September 26, 2023 15:06
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.

5 participants