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 Security Vulnerabilities #113

Merged
merged 14 commits into from
Oct 10, 2024
Merged

Fix Security Vulnerabilities #113

merged 14 commits into from
Oct 10, 2024

Conversation

gurukishorebolisetty
Copy link
Contributor

No description provided.

Copy link
Contributor

@hemantgoswami hemantgoswami left a comment

Choose a reason for hiding this comment

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

I have added comments please have a look at them

Dockerfile Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
hemantgoswami
hemantgoswami previously approved these changes Oct 2, 2024
Copy link
Contributor

@hemantgoswami hemantgoswami left a comment

Choose a reason for hiding this comment

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

Pleaes have a look at the comment

package.json Outdated
"setup": "^0.0.3",
"start": "^5.1.0",
"uglify-js": "^3.15.4",
"winston": "^3.3.3",
"yarn": "^1.22.18"
},
"devDependencies": {
"braces": "^3.0.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

This is only needed in dependencies, as devdependencies are the additional packages for development environment and this will try to install it twice, not that it is a big deal but tidy code by removing it from here will look good.

On the other hand, did you tried updating the parent package that has the dependency on this package to a more recent that should be the ideal solution. The reason for that is, now the package.json suggests that this package is required and used directly by the application and in couple of years time if the parent package is removed and not used, this will still be lying around and no one would know what to do with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hemantgoswami , removed these packages from package.json file now. please resolve and approve PR.

@hemantgoswami hemantgoswami self-requested a review October 2, 2024 20:11
@hemantgoswami hemantgoswami dismissed their stale review October 2, 2024 20:12

Wrong button clicked

Copy link
Contributor

@hemantgoswami hemantgoswami left a comment

Choose a reason for hiding this comment

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

LGTM

@gurukishorebolisetty gurukishorebolisetty merged commit e3a0282 into master Oct 10, 2024
2 checks passed
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