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

properly handle content_type callables returning a single internet media type as scalar #350

Merged
merged 1 commit into from
Mar 3, 2016

Conversation

amotl
Copy link
Contributor

@amotl amotl commented Feb 28, 2016

Improve and resolve #343, add appropriate tests.

@@ -1,5 +1,7 @@
.tox
.hg*
.venv*
.idea
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this? (should it be part of your .gitignore instead?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • .idea is the project metadata folder of PyCharm, which is in pretty common use amongst our folks.
  • When working with python virtualenvs, i think it is a convention to use .venv27 or .venv34 folders for designating the type of virtualenv. Given, when using tox one usually doesn't need manual-made virtualenvs for running the tests, but we usually even install tox into a project-local virtualenv to completely decouple from the system python.

Both entries shouldn't do any harm but instead should gain value for others, so they were added on purpose. However, we can remove them again if they make anyone sad.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe these two lines should not be included because they're specific to one setup. It would make this file harder to maintain over time, for little benefit :-)

@almet
Copy link
Contributor

almet commented Feb 28, 2016

Hi, and thanks for doing this! I've put a bunch of comments here and there, let me know when you had the time to address them!

@amotl
Copy link
Contributor Author

amotl commented Feb 28, 2016

Hey Alex, thanks for reviewing this. Will answer the comments inline.

@amotl
Copy link
Contributor Author

amotl commented Feb 28, 2016

Hi Alex, i tried to clarify my intentions and give an outlook. Let me know if there's still any blocking issue for you in this, otherwise i would appreciate doing a different PR for more cosmetic and cleanup work centered around the things mentioned. Best, Andreas.

@almet
Copy link
Contributor

almet commented Mar 1, 2016

I think we're good once the .gitignore lines have been removed!

@amotl amotl force-pushed the wip/content-type-function branch from 0390a20 to 3a74866 Compare March 3, 2016 02:18
@amotl
Copy link
Contributor Author

amotl commented Mar 3, 2016

Thanks! The .gitignore lines have been removed by a squashed commit.

@almet
Copy link
Contributor

almet commented Mar 3, 2016

Thanks!

almet added a commit that referenced this pull request Mar 3, 2016
properly handle content_type callables returning a single internet media type as scalar
@almet almet merged commit 157b535 into Cornices:master Mar 3, 2016
@amotl
Copy link
Contributor Author

amotl commented Mar 3, 2016

Thanks for merging! Would you also have a look at #351? This complements this change as it improves and clarifies the documentation regarding this topic at the places @bomb-on and @leplatrem were looking at in the context of #343.

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.

content_type not handled properly if it is a function
2 participants