search

Freeseer / freeseer

Designed for capturing presentations at conferences. Pre-fill a list of talks to record, record them, and upload them to YouTube with our YouTube Uploader.
http://freeseer.readthedocs.org
GNU General Public License v3.0
215 stars 110 forks source link

Youtube API refactor for testing #529

Closed MadMub closed 10 years ago

MadMub commented 10 years ago

This PR is to address deficiencies discovered in #460, which calls for more testing "seams" to be present in the code. This will simply refactor the code to be more accessible during testing.

Closes #528

dideler commented 10 years ago

For comments throughout the code, I recommend writing them as proper English sentences. So they would start with a capital letter and end with a period. This is how comments are written in the style guide.

zxiiro commented 10 years ago

I'm not sure what you mean by "Youtube API amendments" can you be more descriptive?

If you're fixing multiple things just split them up into individual patches that describe them exactly and submit the PR. I would prefer we didn't bundle multiple fixes into 1 big PR. If I'm misunderstanding sorry.

zxiiro commented 10 years ago

Can you update your description to describe what this patch actually fixes?

I'm not entirely sure what the point of this PR is from reading the subject and description to be honest.

MadMub commented 10 years ago

Its not multiple fixes, sorry for the confusion I forgot the word testing, its to make the code more easily testable On Mar 25, 2014 11:57 PM, "Thanh Ha" notifications@github.com wrote:

I'm not sure what you mean by "Youtube API amendments" can you be more descriptive?

If you're fixing multiple things just split them up into individual patches that describe them exactly and submit the PR. I would prefer we didn't bundle multiple fixes into 1 big PR. If I'm misunderstanding sorry.

Reply to this email directly or view it on GitHubhttps://github.com/Freeseer/freeseer/pull/529#issuecomment-38647885 .

zxiiro commented 10 years ago

So you're refactoring?

Perhaps the you should reword your subject to indicate that you are "refactoring code" to make it easier to test. If I understand right you're not changing any functionality here?

MadMub commented 10 years ago

There is only very minute control logic changed, for the user nothing is changed I will edit the PR. On Mar 26, 2014 12:14 AM, "Thanh Ha" notifications@github.com wrote:

So you're refactoring?

Perhaps the you should reword your subject to indicate that you are "refactoring code" to make it easier to test. If I understand right you're not changing any functionality here?

Reply to this email directly or view it on GitHubhttps://github.com/Freeseer/freeseer/pull/529#issuecomment-38648505 .

dideler commented 10 years ago

Can you fix whatever's failing in the tests?

dideler commented 10 years ago

Merged in https://github.com/Freeseer/freeseer/commit/ba222ca666c7737d6bd32fe408193056f3657419.