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

Workflow cicd pipeline #12

Merged
6 commits merged into from
Dec 22, 2024
Merged

Workflow cicd pipeline #12

6 commits merged into from
Dec 22, 2024

Conversation

Hermann27
Copy link
Collaborator

This PR adds a full CI/CD pipeline for the project, which includes the following:

  • Node.js and npm setup: Installs Node.js and dependencies.
  • Test workflow: Runs tests whenever code is pushed to 'main' or on pull requests.
  • Deployment workflow: Deploys the project to xyz after successful tests.

This PR adds a full CI/CD pipeline for the project, which includes the following:
- **Node.js and npm setup**: Installs Node.js and dependencies.
- **Test workflow**: Runs tests whenever code is pushed to 'main' or on pull requests.
- **Deployment workflow**: Deploys the project to xyz after tests are successful.

The workflow file is located in `.github/workflows/ci-cd-pipeline.yml`.
@robstrader
Copy link
Collaborator

@Hermann27 the actions look good, but @ozzydogandev has now merged in his changes for the homepage, so now the example test config is broken. You will need to update this branch with the latest, then fix the issue. The error is that jest does not know how to handle .css files. Please see the following section of the docs: https://jestjs.io/docs/webpack#handling-static-assets

You may even want to fix that in a separate pull request, then update this PR after that one has been merged. Or you can fix it in this one.

@robstrader
Copy link
Collaborator

To update your PR branch, you have 2 choices, you can rebase or merge locally, then push to the remote branch, or you can update the branch remotely. Updating remotely is fairly simple when there are no merge conflicts, you can simply click the 'Update branch' button in the PR. Then do a hard reset on your local branch to the remote branch:

update-branch-pr

After clicking 'Update branch':

git checkout pr-mybranch
git fetch
git reset --hard origin/pr-mybranch

Hermann27 and others added 2 commits December 17, 2024 08:28
- Fixed outdated branch by merging `main` into the feature branch to sync with the latest changes.
- Resolved Jest configuration issues to properly handle CSS and style imports in tests.
- Configured ESLint to automatically fix default formatting issues such as indentation, semicolons, and spacing, ensuring consistent code style across the project.
@Hermann27
Copy link
Collaborator Author

PR updated 👍

Copy link
Collaborator

@robstrader robstrader left a comment

Choose a reason for hiding this comment

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

Overall, looks great! Added a couple of comments. Good job on getting Jest working with CSS files.

package.json Outdated
"lint": "eslint . --report-unused-disable-directives --max-warnings 0",
"lint": "eslint . --fix --report-unused-disable-directives --max-warnings 0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure you actually want to use --fix here. You would like lint to report an errors. You could create a separate script for fixing named lint:fix.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have added a separate script for fixing the lint.

jest.config.mjs Outdated
Comment on lines 4 to 9
/* transform: {
"^.+\\.jsx?$": "babel-jest",
},
moduleNameMapper: {
moduleNameMapper: {
"^@/(.*)$": "<rootDir>/src/$1",
},
},*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you go ahead and remove the commented out lines.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have removed the comment as requested.

Copy link
Collaborator

@robstrader robstrader left a comment

Choose a reason for hiding this comment

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

Sorry, meant to get back to this. Looks good to me, but you are going to need to merge in the latest changes from main as Ozzy has been pushing some changes.

@Hermann27
Copy link
Collaborator Author

image

I have updated this PR with the latest changes as Ozzy has pushed some changes. Now I need to fix all those eslint error

@Hermann27
Copy link
Collaborator Author

for now, I have turned off these rules:

  • "react/prop-types": "off",

  • "react/react-in-jsx-scope": "off",

  • "react/no-unescaped-entities": "off",

What would be the best way to correct those errors?

@Hermann27 Hermann27 closed this pull request by merging all changes into main in e817338 Dec 22, 2024
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.

2 participants