The-Enthusiast-404 / git-begin

Git Begin is a Web App to find latest good first issues and start contributing to open source
https://gitbegin.theenthusiast.dev
MIT License
28 stars 15 forks source link

Fix: Change Footer layout #73

Closed rodriveiga01 closed 2 weeks ago

rodriveiga01 commented 2 weeks ago

Summary

This pull request has changes based on issue #70 , that is about changing layout of Footer

Changes

Image

New Footer

Resolved Issues

Resolves #70

netlify[bot] commented 2 weeks ago

Deploy Preview for gitbegin ready!

Name Link
Latest commit cc6cb458003f338b5ef38f5888f5cd7edf002189
Latest deploy log https://app.netlify.com/sites/gitbegin/deploys/66d5ad41e51aa7000850e467
Deploy Preview https://deploy-preview-73--gitbegin.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

Lighthouse
1 paths audited
Performance: 70
Accessibility: 90
Best Practices: 83
SEO: 88
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

omjogani commented 2 weeks ago

@rodriveiga01 Would you mind sharing a screenshot of the warning you're getting without removing first 2 lines in .husky?

omjogani commented 2 weeks ago

@rodriveiga01 Please also mention, what vulnerabilities you found that led you to modify package.lock file?

dotslashbit commented 2 weeks ago

@omjogani thanks for checking this out, let me know when everything is done, I'll merge it

rodriveiga01 commented 2 weeks ago

Ok i will share all of that

rodriveiga01 commented 2 weeks ago

Because it found some warnings of vulnerabilities and install command is deprecated, I used the "npm update" and it updated everything and it then didn't have warnings and it worked like normal

Captura de ecrã 2024-09-02, às 10 13 10
rodriveiga01 commented 2 weeks ago

The warning from husky was this:

Please remove the following lines from ./.husky/pre-commit:

#!/usr/bin/env sh
. \"\$(dirname -- \"\$0\")/_/husky.sh\"

They WILL FAIL in v10.0.0
Captura de ecrã 2024-09-02, às 10 19 48

This appears in the release of the version 9.0.1 which says that the two lines can and need to be removed

rodriveiga01 commented 2 weeks ago

Also when I ran "npm run dev" it would show me an error before updating the package.lock:

Captura de ecrã 2024-09-02, às 10 54 33

After I ran npm update it would work

rodriveiga01 commented 2 weeks ago

If all of you agree I think it would be good to change the README.md because there it says to run "npm install" but that command is deprecated. It would be needed to change to "npm update"

dotslashbit commented 2 weeks ago

@omjogani Please check this, thanks

omjogani commented 2 weeks ago

@rodriveiga01 Here is what's going on!

When I configured Husky, It was working well and then There was migration from Husky which was leading to the state that "install is DEPRECATED". but the issue is we haven't migrated the configuration to the latest one. There is no requirement of updating dependencies in my opinion.

You don't really have to run npm update command, just follow below step

  1. Take out changes you've done in the project apart from configuration (FooterFile)
  2. Reset this branch to main branch
  3. Remove those 2 line in .husky file, New content will be
    npx lint-staged
  4. In package.json file, in the script update prepare script Old: "prepare": "husky install" New: "prepare": "husky"
  5. Add your changes (Footer file)
  6. Test out everything is working
  7. push it again
omjogani commented 2 weeks ago

Migration Guide: https://github.com/typicode/husky/releases/tag/v9.0.1

Let me know if you need any help! @rodriveiga01

rodriveiga01 commented 2 weeks ago

@omjogani Thanks a lot for the instructions. Its all working as expected

rodriveiga01 commented 2 weeks ago

I did a force push to reset the changes. I don't know if it's the best practice but was what I knew

dotslashbit commented 2 weeks ago

@rodriveiga01 Thank you so much for your contribution. Let me just get the confirmation from @omjogani and then I'll merge it.

@omjogani Just let us know if its good to go or not, thank you

rodriveiga01 commented 2 weeks ago

Ok sounds good

omjogani commented 2 weeks ago

@dotslashbit Merge this please!