search

Moorad / youtube-video-downloader

199 stars 138 forks source link

my changes #21

Closed balajinikhil closed 4 years ago

balajinikhil commented 4 years ago

I have added drop down box for selecting mp3 or mp4 instead of two buttons, The filename will be obtained from "ytdl.getBasicInfo", the youtube video name will be the filename, If there was an error while extracting filename then it will be audio/video according to the file type. This is my first contribution to any project, Thank You

Moorad commented 4 years ago

Thank you so much for your contribution. Functionality-wise the code looks perfectly fine and I'm happy to merge it but there is some code inconsistency compared to the old code. One of them is the use of double quotes ("), this is purely a preference thing but the original code used single quotes (') rather than double quotes. Another inconsistency is spacing, You used 2 spaces to indicate an indentation. Replace that with a single tab (which is 4 spaces wide) for an indentation. Lastly is switching the order of app.get('/downloadmp4', ... app.get('/downloadmp3', .... very simple to fixes and I will be happy to merge after that. Sorry for these requests that might seem meaningless but it's just to keep the code as close in the format of the blog as possible.

Moorad commented 4 years ago

Also, you have mentioned that you have changed the two buttons to a drop down but I think you forgot to push those changes. The only changes you've pushed were to Server/index.js

balajinikhil commented 4 years ago

Hi, I have done the changes you have asked, I'm not familiar with changing indentation, I have done to the best of my knowledge, Thank You

Moorad commented 4 years ago

I will test and format the html, css and js side of the code now

Moorad commented 4 years ago

Sorry for all of the requested changes ;-;

balajinikhil commented 4 years ago

Hi, I have done changes you had asked to, If there is anything else please feel free to tell. Thank You.

Moorad commented 4 years ago

why did you change everything back to double quotes?

balajinikhil commented 4 years ago

Sorry my code editor I had enabled it after previous change, I'm really sorry, I'll fix that

Moorad commented 4 years ago

no no don't worry about it. Right now the code is in a good state. I will merge it and then create another pull request to format it

Moorad commented 4 years ago

But thank you so much for the contribution, it means a lot. you've been very nice. Don't be afraid to open another pull request if you want to improve this even more

Moorad commented 4 years ago

oops