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

refactor repo structure #3022

Closed
mmerickel opened this issue May 2, 2017 · 13 comments · Fixed by #3388
Closed

refactor repo structure #3022

mmerickel opened this issue May 2, 2017 · 13 comments · Fixed by #3388
Milestone

Comments

@mmerickel
Copy link
Member

  1. Move tests out of the pyramid package. They will still be shipped with the sdist but not with the binary wheel.

  2. Move the pyramid package into a src subfolder in the repo to conform to best practices in other repos like hupper and pyramid_retry. This is a simple fix to prevent pyramid source from being on the PYTHONPATH inadvertently which means we're testing the source instead of the actual sdist built by tox. This also avoids subtle issues in which multiple versions of python are run against the same code.

@mmerickel mmerickel added this to the 2.0 milestone May 2, 2017
@mcdonc
Copy link
Member

mcdonc commented Jul 3, 2018

+1 on both

@mmerickel mmerickel modified the milestones: 2.0, 1.10 Jul 30, 2018
@mmerickel
Copy link
Member Author

for anyone doing this work, please don't do it until at least #3113 is done

@mmerickel
Copy link
Member Author

@mcdonc @tseaver anyone else... are any of you opposed to me running black on the code and adding black --check -l 79 src/pyramid tests to tox -e lint? if not then I'm going to do it as part of this refactor. I know that at least @bertjwregeer and I are a big +1 on it.

@mcdonc
Copy link
Member

mcdonc commented Oct 7, 2018

+0

@digitalresistor
Copy link
Member

I'd want to leave the default line length at 88... but I can live with 79, so long as we add a pyproject.toml that includes that as a default setting so I don't have to remember to set it in my editor :-).

@mcdonc
Copy link
Member

mcdonc commented Oct 7, 2018

I'm skeeved out by its replacement of ' with ". But whatever.

@mmerickel
Copy link
Member Author

Yeah that’s by far my biggest gripe with it as well. But I’m not sure it’s worth throwing out the whole thing as a result. I’m open to setting the cli flag that preserves quoting but the more flags we set the more ways people can screw it up.

@mcdonc
Copy link
Member

mcdonc commented Oct 7, 2018

Given that it's a flag already (it's --skip-string-normalization right?) I might say set it, f it. :)

@mcdonc
Copy link
Member

mcdonc commented Oct 7, 2018

When I look at the diff from black -l 79 -S pyramid I don't see anything that I couldn't live with.

@digitalresistor
Copy link
Member

Setting that flag means that no quoting gets modified, which means things like:

{'some': "dict", "other": 'whatever'}

Is okay...

That bothers me, I'd prefer it just normalized it for me.

Thankfully we can add flags to pyproject.toml which will be picked up automatically by a black run, so we can avoid people "screwing" it up. Same with the line length.

@mmerickel
Copy link
Member Author

the refactoring is done in #3387 and the black config is done in #3388

@Cykooz
Copy link
Contributor

Cykooz commented Nov 23, 2018

How can I use Pyramid tests after that change?

I have a project with extended version of pyramid.authorization.ACLAuthorizationPolicy. I am using TestACLAuthorizationPolicy test case to testing my implementation:

from pyramid.tests.test_authorization import TestACLAuthorizationPolicy

class TestRestACLAuthorizationPolicy(TestACLAuthorizationPolicy):

    def _getTargetClass(self):
        from ..authorization import RestACLAuthorizationPolicy
        return RestACLAuthorizationPolicy

    # Some addition tests for extended functionality

It would be cool if you upload into PyPi package with all Pyramid tests, e.g. pyramid.tests.

@digitalresistor
Copy link
Member

@Cykooz the pyramid tests were never considered public API, and if you want to sub-class them you should copy and paste them into your project.

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 a pull request may close this issue.

4 participants