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

Added channel info (i.e. [25.1]) to the filename. #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jschwalbe
Copy link
Contributor

Channel info to help with sorting or automated commercial removal tools (i.e. comskip)

Channel info to help with sorting or automated commercial removal tools (i.e. comskip)
@tdickman
Copy link
Owner

Thanks for sending this over, I appreciate the help! You're using ".encode('utf-8')" on parts of the string, but not on others. Any specific reason?

@jschwalbe
Copy link
Contributor Author

It was that way when I started, and was afraid to get rid of it. :)

I can’t recall but I think that I tried removing it and it caused a problem.. but then I moved on to other stuff, so can’t recall exactly. Probably could go!

On Tue 8/12/14, at 9:53 PM, tdickman notifications@github.com wrote:

Thanks for sending this over, I appreciate the help! You're using ".encode('utf-8')" on parts of the string, but not on others. Any specific reason?


Reply to this email directly or view it on GitHub.

@tdickman
Copy link
Owner

I can take a look at it again later, and see if there is a better way, but I recommend getting rid of it.

Also, I went through and cleaned up some of the code to fall better in line with python coding standards (to keep everything consistent). A few comments:

  1. Modify your editor to use 4 spaces instead of tabs
  2. You typically shouldn't comment out code - just delete it since the old version will stay in the git repo for you to look at. It makes the code look a lot cleaner, and it's much easier to read.
  3. Use function and variable names like this: count_the_stars() instead of: countTheStars.

It probably looks like I made a ton of changes to your code, but I mainly changed tabs to spaces, etc. It looks like all of the lines changed since I swapped tabs to spaces. No hard feelings, I know you said you are new to python coding - I'm not trying to be difficult, just trying to keep all of the files consistent :).

@jschwalbe
Copy link
Contributor Author

No problem. It looks good! Thanks for the pointers ;)

Question — did the channel info in the filename get deleted on purpose? If so I’m just going to fork the project and keep it forked because it’s something I’m going to be relying upon with my automation.

On Tue 8/12/14, at 10:37 PM, tdickman notifications@github.com wrote:

I can take a look at it again later, and see if there is a better way, but I recommend getting rid of it.

Also, I went through and cleaned up some of the code to fall better in line with python coding standards (to keep everything consistent). A few comments:

  1. Modify your editor to use 4 spaces instead of tabs
  2. You typically shouldn't comment out code - just delete it since the old version will stay in the git repo for you to look at. It makes the code look a lot cleaner, and it's much easier to read.
  3. Use function and variable names like this: count_the_stars() instead of: countTheStars.

It probably looks like I made a ton of changes to your code, but I mainly changed tabs to spaces, etc. It looks like all of the lines changed since I swapped tabs to spaces. No hard feelings, I know you said you are new to python coding - I'm not trying to be difficult, just trying to keep all of the files consistent :).


Reply to this email directly or view it on GitHub.

@tdickman
Copy link
Owner

Nope, that was a mistake - if you can make update you branch (with the newest changes), and send another pull request, I can merge it back in. Sorry about that!

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