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

#2897 Adding tests for pie ccw. Issue 2897 #2933

Merged
merged 4 commits into from Mar 28, 2014

Conversation

sfroid
Copy link
Contributor

@sfroid sfroid commented Mar 28, 2014

No description provided.

@sfroid sfroid mentioned this pull request Mar 28, 2014
@sfroid sfroid closed this Mar 28, 2014
@sfroid sfroid deleted the issue_2897 branch March 28, 2014 01:13
@tacaswell
Copy link
Member

Why did you delete this?

We probably only need to test the non-default case as the existing tests cover the default case.

@sfroid
Copy link
Contributor Author

sfroid commented Mar 28, 2014

Sorry. I will make another pull request. I believed this was the wrong branch and acted fast. Too fast I guess.

@sfroid sfroid restored the issue_2897 branch March 28, 2014 18:51
@tacaswell tacaswell reopened this Mar 28, 2014
@tacaswell
Copy link
Member

I reopened it for you.

Don't worry too much about making mistakes. We are all pretty friendly/understanding here and unless it is merged into master it won't hurt anyone.

@tacaswell
Copy link
Member

You shouldn't be removing the line width test files...

@sfroid
Copy link
Contributor Author

sfroid commented Mar 28, 2014

About removing the line width test files - we should not remove them in this pull request or we should not do that at all?

I imagined that the png test would be enough and that the pdf / svg tests were superfluous and we could do without them. I have only removed the pdf / svg files. The png are still there.

@tacaswell
Copy link
Member

Ah, sorry I didn't read carefully enough. I missed that it was just the not-png images. Sorry for the noise.

@sfroid
Copy link
Contributor Author

sfroid commented Mar 28, 2014

Cool!
In that case, do we need any more work on this pull request or is this good to go?
It has the tests, test files and the changelog.

tacaswell added a commit that referenced this pull request Mar 28, 2014
closes #2897 Adding tests for pie ccw. Issue 2897
@tacaswell tacaswell merged commit 998676a into matplotlib:master Mar 28, 2014
@tacaswell
Copy link
Member

Merged. Thank you @sfroid !

@sfroid
Copy link
Contributor Author

sfroid commented Mar 28, 2014

Yayy... second contribution! Thanks for the help @tacaswell

@sfroid sfroid deleted the issue_2897 branch March 28, 2014 21:57
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.

None yet

2 participants