python-20 / video-downloader

A python application to download videos
GNU General Public License v3.0
10 stars 10 forks source link

Update Read-me and mac-issue with Certifi #33

Closed manjilgautam closed 4 years ago

chonix commented 4 years ago

@manjilgautam: I have a few suggestions here. Seems you have done some tasks and we certainly have to look throughout the code. I'd suggest that you do

also , we need to update .gitignore file to add .DS_Store so it gets ignored when you pull the repo (I believe this is a file from macos or similar to the thumbs.db in windows or something like that)

Not bad, but I'd break that down cause it's a good practice :) (and sorry if I'm being too straightforward)

manjilgautam commented 4 years ago

Thanks for the constructive feedback. I will definitely note these points before making new pull requests.

cherylli commented 4 years ago

hey, thank you for contributing but i think you have a slightly outdated version of the repo, possibly because you forked it before some changes were made.

It's probably not a big issue as we can ressolve conflicts when we merge but its a good practice to update the branch before editing it.

You can try to clone master directly, create branches, and just do a pull request without forking. That way you can always just pull the lastest version before you start!

I should also remove pyside2.py as it is not required anymore.

Correct me if i'm wrong it says 6 files updated but the only changes are in readme.md? and a test case? Just want to make sure i didn't miss any changes

manjilgautam commented 4 years ago

Sure! I will pull it from the master branch from now onwards.

cherylli commented 4 years ago

Will close this for now as @manjilgautam wants to redo it