-
Notifications
You must be signed in to change notification settings - Fork 11
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
Added E Commerce website #6
Conversation
@rahulch-1 Please make the required changes to pass the lining checks, so that we can review and merge your PR. And don't worry about the files that end with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make the requested changes, so that we can perform another code review to approve your PR to merge.
@rahulch-1 don't worry about the lining checks for now, as the only issues that are reported for creating unused variables (which were used in other files) and not recognising the jquery syntax. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rahulch-1 The only changes that were made to resolve the code review issue is to remove a single .DS_Store
file from your projects. You still need to make the following changes:
- Remove the directory/file names
.github
. - Remove all
.DS_Store
files. - Reply to the previously mentioned comments about
openhand.html
andclose hand.html
(You can find them in our PR conversation)
Note
Don't worry about the linting checks as we're ignoring them for your case. Just make the above mentioned changes.
Now I removed the .github directory and all the .DS_Store files also |
Thanks for helping me out |
@rahulch-1 you have indeed removed |
All of these remaining |
Sorry, but I am not able to find any .DS_store files now |
It's okay @rahulch-1. Just so you know, this linter makes sure each and every file maintains its integrity and works as intended without any syntactical errors. In some cases, the linter that we have used checks purely for JavaScript and doesn't recognise any external framework. This is the reason why people set up linter within their projects that are built using frameworks, as these linter will be configured according to the framework avoiding any linting issues. And the way we configured our linter for this repo is for pure javascript files and not medium to big projects built using frameworks like React. We respect your patience and willing to contribute, but I, @iamwatchdogs, as the main maintainer have to make sure you give the best from your side while maintaining the whole integrity of the repo. So, please have some patience as when we request to make a few changes. Hope this helps you understand what we at @Grow-with-Open-Source are trying to do. |
Sorry for that @rahulch-1, Yes all the files were removed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Request Changes were made. Reviewed and Approved by @iamwatchdogs.
yes, @iamwatchdogs I completely understand the need for a specialized linter setup |
Thank you for contributing, @rahulch-1. Please check your contribution on GitHub Pages. |
This is an E-Commerce Website made using HTML, CSS, JavaScript which has products with their description and their reviews.