Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Reduce Travis verbosity and remove most LGTM issues #258

Merged
merged 22 commits into from
Apr 10, 2019
Merged

Conversation

andrrizzi
Copy link
Collaborator

Fix #234 and remove almost all LGTM issues. The remaining LGTM issues are related to environment.py, which we're not sure if it will kept around, and gbsaforces.py, which I believe is still largely unsupported.

@j-wags
Copy link
Member

j-wags commented Apr 9, 2019

This pull request introduces 1 alert and fixes 61 when merging a8205cc into 0b4c523 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

fixed alerts:

  • 17 for Unused import
  • 13 for Unused local variable
  • 11 for Testing equality to None
  • 10 for Unreachable code
  • 6 for 'import *' may pollute namespace
  • 1 for Use of the return value of a procedure
  • 1 for Suspicious unused loop iteration variable
  • 1 for Module is imported with 'import' and 'import from'
  • 1 for Variable defined multiple times

Comment posted by LGTM.com

@j-wags
Copy link
Member

j-wags commented Apr 9, 2019

This pull request fixes 61 alerts when merging a3c1ad2 into 0b4c523 - view on LGTM.com

fixed alerts:

  • 17 for Unused import
  • 13 for Unused local variable
  • 11 for Testing equality to None
  • 10 for Unreachable code
  • 6 for 'import *' may pollute namespace
  • 1 for Use of the return value of a procedure
  • 1 for Suspicious unused loop iteration variable
  • 1 for Module is imported with 'import' and 'import from'
  • 1 for Variable defined multiple times

Comment posted by LGTM.com

@codecov-io
Copy link

codecov-io commented Apr 10, 2019

Codecov Report

Merging #258 into master will increase coverage by 1.67%.
The diff coverage is 57.4%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #258      +/-   ##
==========================================
+ Coverage    73.1%   74.78%   +1.67%     
==========================================
  Files          17       17              
  Lines        5128     5012     -116     
==========================================
- Hits         3749     3748       -1     
+ Misses       1379     1264     -115
Impacted Files Coverage Δ
openforcefield/utils/serialization.py 91.3% <ø> (+6.45%) ⬆️
openforcefield/typing/engines/smirnoff/io.py 64.07% <100%> (-0.35%) ⬇️
...enforcefield/typing/engines/smirnoff/forcefield.py 67.84% <100%> (+0.12%) ⬆️
openforcefield/topology/topology.py 71.34% <100%> (+0.1%) ⬆️
openforcefield/utils/utils.py 91.13% <100%> (+0.05%) ⬆️
openforcefield/topology/molecule.py 86.24% <33.33%> (+2.38%) ⬆️
openforcefield/utils/structure.py 43.59% <50%> (ø) ⬆️
openforcefield/typing/chemistry/environment.py 62.11% <66.66%> (+0.06%) ⬆️
...enforcefield/typing/engines/smirnoff/gbsaforces.py 21.95% <66.66%> (-2.76%) ⬇️
openforcefield/utils/toolkits.py 86.43% <66.66%> (+5.71%) ⬆️
... and 1 more

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 0b4c523...a3c1ad2. Read the comment docs.

@jchodera
Copy link
Member

Wow, awesome!

Copy link
Member

@jchodera jchodera left a comment

Choose a reason for hiding this comment

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

Looks good! Test verbosity is much better now.

@andrrizzi andrrizzi requested a review from j-wags April 10, 2019 00:52
@andrrizzi
Copy link
Collaborator Author

Thank you!

@andrrizzi andrrizzi merged commit 236456d into master Apr 10, 2019
@andrrizzi andrrizzi deleted the fix-lgtm branch April 10, 2019 00:59
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants