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

NW5 Manchester - Doris Siu - SQL - Week 1 #245

Closed
wants to merge 34 commits into from

Conversation

Doris-Siu
Copy link

No description provided.

The previous PR tracked node_modules even I have included .gitignore. I found out that I have forked the repo which the node_modules was commited, caused the issue. Therefore, I do the work from scratch to make sure the forked  repo I work on is removed with node_modules.
});

const handleChange = (e) => {
setInput({ ...input, [e.target.name]: e.target.value });
Copy link

Choose a reason for hiding this comment

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

Looks good, just a minor suggestion, would it be possible to destructure name and value from e.target as variables?

addVideo(input);
};

function uuidv4() {
Copy link

@Craques Craques Feb 9, 2023

Choose a reason for hiding this comment

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

maybe you can use the uuid library for this unless there is a valid reason to build your own random character generator, otherwise this looks great, good stuff

https://www.npmjs.com/package/uuid if you are using npm
https://yarnpkg.com/package/uuidv4 uf you are using yarn

@Dedekind561 Dedekind561 closed this Apr 8, 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.

3 participants