Skip to content

Some minor style improvements and 2 test fixes - #1330

Merged
nicolaiarocci merged 3 commits into
pyeve:masterfrom
stefaang:stylefix
Nov 29, 2019
Merged

nicolaiarocci merged 3 commits into
pyeve:masterfrom
stefaang:stylefix

Conversation

@stefaang

Copy link
Copy Markdown
Contributor
  • fixed the 401 or 405 assertion always returning True
  • tests were failing on a fresh install without test_user
  • I made it a bit more pythonic.

@nicolaiarocci

Copy link
Copy Markdown
Member

excellent. Tests are in dire need of refactoring (if not a complete rewrite).

@nicolaiarocci nicolaiarocci added this to the 0.10 milestone Nov 26, 2019
nicolaiarocci added a commit that referenced this pull request Nov 29, 2019
@nicolaiarocci
nicolaiarocci merged commit 8816e2c into pyeve:master Nov 29, 2019
@nicolaiarocci

Copy link
Copy Markdown
Member

Thanks.

Comment thread eve/methods/get.py

for key, value in query.items():
if key[0] != "$":
if key.startswith("$"):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Was this change intended or should it be if not key.startswith("$"):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hmm not intended.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess this code path isn't being tested?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the review @mic-max. Feel free to PR a fix.

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.

4 participants