search

plone / plone.app.testing

Testing tools for Plone-the-application
http://pypi.python.org/pypi/plone.app.testing
2 stars 7 forks source link

Do not use install Products.SecureMailHost in the PloneFixture if it isn't available #28

Closed vangheem closed 8 years ago

vangheem commented 8 years ago

tests: http://jenkins.plone.org/job/pull-request-5.0/1066/

gforcada commented 8 years ago

Totally unrelated but thanks anyway: you uncovered a bug on the package QA jenkins job :-)

gforcada commented 8 years ago

@vangheem and as it is fixed now, would you mind reformatting that line to be shorter than 80 characters? Thanks in advance!

vangheem commented 8 years ago

ok, an aside, can we increase that limit to more than 80? I thought it wasn't really recommended anymore and that we've moved on to longer line lengths.

thet commented 8 years ago

After discussing the styleguide about documentation here and there, I also started using the ReST styleguide for changelog files ( https://github.com/plone/documentation/blob/5.0/about/rst-styleguide.rst#id4 ). It says to have one sentence per line and line length doesn't matter. It's better for translators to translate text. But wrapping any kind of text within the 80 columns limit is cumbersome.

+1 for giving up the 80 chars restriction for any kind of docs, except for docstrings or the like in Python files.

gforcada commented 8 years ago

@vangheem @thet can we just stick to what has been endlessly discussed? And sorry if it feels a bit rude, but as a community we already decided on sticking to full pep8.

This re-formatted string (as already done, thanks @vangheem ) is still perfectly readable.

thet commented 8 years ago

@gforcada I don't think PEP 8 is meant to be applied to non-python files. Even our Python styleguide is explicit about this here: https://github.com/plone/documentation/blob/5.0/develop/styleguide/python.rst#line-length

[the 79 column line length rule] explicitly does not aply to documentation .rst files. For rst files, use semantic linebreaks.

But that's not related and I move this discussion to here: https://github.com/plone/documentation/issues/548

gforcada commented 8 years ago

@thet sure, i was only talking about python code here (as it was what jenkins reported to this pull request).

thet commented 8 years ago

oh! sorry for the confusion. I only saw the changelog exceeding 80 cols.

vangheem commented 8 years ago

Yah, okay, just a huge pet peeve of mine and I think it's rather insane.