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

Fix exception arguments and documentation #198

Closed
wants to merge 1 commit into from

Conversation

hluk
Copy link

@hluk hluk commented Jul 23, 2019

Signed-off-by: Lukas Holecek hluk@email.cz

@pypingou
Copy link
Member

Looks good to me, though I expect we may want tests with this?

@codecov-io
Copy link

codecov-io commented Jul 23, 2019

Codecov Report

Merging #198 into master will decrease coverage by 15.51%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #198       +/-   ##
==========================================
- Coverage   95.42%   79.9%   -15.52%     
==========================================
  Files          13      13               
  Lines        1312    1324       +12     
  Branches      158     160        +2     
==========================================
- Hits         1252    1058      -194     
- Misses         45     244      +199     
- Partials       15      22        +7
Impacted Files Coverage Δ
fedora_messaging/exceptions.py 94.44% <100%> (+1.58%) ⬆️
fedora_messaging/twisted/consumer.py 45.94% <0%> (-54.06%) ⬇️
fedora_messaging/twisted/factory.py 58.08% <0%> (-35.33%) ⬇️
fedora_messaging/twisted/protocol.py 60.84% <0%> (-32.37%) ⬇️
fedora_messaging/api.py 79.54% <0%> (-20.46%) ⬇️
fedora_messaging/twisted/service.py 92.8% <0%> (-4.8%) ⬇️
fedora_messaging/message.py 94.21% <0%> (-1.24%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f76c9e8...f3c1648. Read the comment docs.

@hluk
Copy link
Author

hluk commented Jul 23, 2019

Any idea what's up with the integration test? For me they get also stuck locally.

Copy link
Member

@jeremycline jeremycline left a comment

Choose a reason for hiding this comment

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

I think the CI is fixed in #194. @abompard, are you around to review that?

fedora_messaging/exceptions.py Outdated Show resolved Hide resolved
fedora_messaging/exceptions.py Outdated Show resolved Hide resolved
@jeremycline
Copy link
Member

Actually, current master also passes CI. Maybe one of the tests is expecting something from one of these exceptions that broke? I don't have time to dig into it today, but I might be able to in the next couple days.

Signed-off-by: Lukas Holecek <hluk@email.cz>
@hluk hluk closed this Nov 11, 2019
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