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 #46 Recommended lgtm python issues #96

Merged
merged 1 commit into from
Jul 1, 2020

Conversation

lbleier-GSFC
Copy link
Contributor

Describe the contribution

Testing performed
Steps taken to test the contribution:
Pending; need to do lgtm run

Expected behavior changes
No lgtm issues reported in Python code

Contributor Info - All information REQUIRED for consideration of pull request
Leor Bleier, NASA GSFC\Code 582

@lbleier-GSFC lbleier-GSFC changed the base branch from master to integration-candidate May 27, 2020 20:40
@lgtm-com
Copy link

lgtm-com bot commented May 27, 2020

This pull request fixes 3 alerts when merging 257f929 into 5c14375 - view on LGTM.com

fixed alerts:

  • 1 for Unused local variable
  • 1 for Except block handles 'BaseException'
  • 1 for Unreachable code

@lgtm-com
Copy link

lgtm-com bot commented May 27, 2020

This pull request fixes 3 alerts when merging 257f929 into b7b7ad1 - view on LGTM.com

fixed alerts:

  • 1 for Unused local variable
  • 1 for Except block handles 'BaseException'
  • 1 for Unreachable code

@skliper skliper linked an issue Jun 1, 2020 that may be closed by this pull request
@skliper skliper added the enhancement New feature or request label Jun 1, 2020
@skliper skliper added this to the 2.2.0 milestone Jun 1, 2020
@astrogeco
Copy link
Contributor

@lbleier-GSFC is this ready?

@lbleier-GSFC
Copy link
Contributor Author

@lbleier-GSFC is this ready?

If lgtm runs without issue, then yes :)

@astrogeco astrogeco added conflict CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Jun 24, 2020
@astrogeco
Copy link
Contributor

@lbleier-GSFC is this ready?

If lgtm runs without issue, then yes :)

Can you rebase and see if these conflicts get resolved?

Problems shown in this issue were fixed in nasa#72 as part of
updates/refactoring. Other lgtm issues addressed here.
Problems in auto-generated .py files not addressed.
Auto-generated .py files renamed with Ui_ prefix.
The lgtm.yml file must be updated to exclude these
See nasa/cFS#92
@skliper
Copy link
Contributor

skliper commented Jun 24, 2020

I'm going to need an explanation of what this is supposed to implement... if LGTM is ignoring these files from the cFS yml script I'm not sure what this is supposed to do.

@astrogeco astrogeco added CCB-20200624 and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Jun 24, 2020
@astrogeco
Copy link
Contributor

CCB-20200624: Will investigate further at the bundle level to see if files are ignored by lgtm.

@astrogeco astrogeco added CCB:Approved Indicates approval by CCB IC-20200624 and removed conflict labels Jul 1, 2020
@astrogeco astrogeco merged commit aa15da9 into nasa:integration-candidate Jul 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCB:Approved Indicates approval by CCB enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Recommended lgtm python issues
3 participants