Skip to content

Conversation

Midnighter
Copy link

This is an easy implementation for the feature discussed in #6.

@wolph
Copy link
Owner

wolph commented May 12, 2014

Thank you for the addition @Midnighter, I can't merge the pull request yet because it breaks the build ( https://travis-ci.org/WoLpH/python-progressbar/builds/24969677 ). This repository has a 100% test coverage requirement so all new code needs to have a test as well.

You can add tests if you want, otherwise I'll do it as soon as I'll have a bit more time (probably in about a week).

@Midnighter
Copy link
Author

Can't say that I see the value of having 100% coverage when your tests are really just examples but I guess it's a good idea to add examples for the new behavior anyway.

@wolph
Copy link
Owner

wolph commented May 12, 2014

They serve as both examples and as a simple test that the basics work. If something would be broken, the code wouldn't be callable anymore and the tests would break.

The tests aren't the highest quality indeed, but at least we know that every line of code is being run and therefore working for at least 1 use-case.

@Midnighter
Copy link
Author

So would you like me to add more examples at the end and keep the numbering or intersperse existing examples with alternative versions that use the new __iadd__?

@wolph
Copy link
Owner

wolph commented May 12, 2014

Just an additional example that shows the usage of the new operator should do the trick :)

Basically the code that you showed in #6 should work I think.

* add two new examples `example24` and `example25`
@Midnighter
Copy link
Author

Strangely, Travis now complains about lines 304-307 which I didn't even touch. They are within the update method, though.

if self.redirect_stdout and sys.stdout.tell():
    self.fd.write('\r' + ' ' * self.term_width + '\r')
    self._stdout.write(sys.stdout.getvalue())
    self._stdout.flush()
    sys.stdout = StringIO()

@wolph
Copy link
Owner

wolph commented May 12, 2014

@Midnighter
Copy link
Author

Yeah, pyflakes was showing it as an exact duplicate. Both by function name and the content looked the same, too. Maybe I was just tired, though. I'll try with putting it back.

@Midnighter
Copy link
Author

Guess I was tired, didn't notice that one case was treating stdout and the other stderr. I have renamed them to versions a and b in order to avoid a conflict.

@wolph
Copy link
Owner

wolph commented May 12, 2014

Awesome, thank you very much for the pull request :)

I'll push out a new version as well

wolph added a commit that referenced this pull request May 12, 2014
allow inplace addition to update currval
@wolph wolph merged commit 14ba346 into wolph:master May 12, 2014
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