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

Switch from LGTM to GitHub code scanning #1550

Closed
AdamRJensen opened this issue Sep 13, 2022 · 5 comments
Closed

Switch from LGTM to GitHub code scanning #1550

AdamRJensen opened this issue Sep 13, 2022 · 5 comments

Comments

@AdamRJensen
Copy link
Member

AdamRJensen commented Sep 13, 2022

Describe the bug
On pvlib's GitHub landing page (readme file), there's a shield/link to LGTM. While following this link, I noticed the following warning message:

LGTM.com will be shut down in December 2022 — we recommend that you use GitHub code scanning instead. Read more in our blog post on the GitHub blog.

So sometime before December, we should address this issue by either:

  • removing the LGTM shield from the readme file
  • switch to GitHub code scanning
  • find a third alternative

Upon reading the blog post linked in the message, it seems that the best option might be to just wait:

October: help migrate repositories to GitHub code scanning
We will do our best to help migrate repositories that actively use LGTM.com to flag potential security issues in their pull requests. For those repositories, we will create pull requests that add a GitHub Actions workflow that runs code scanning. Once that configuration file is merged, the repository’s source code (and future pull requests) will be scanned by GitHub code scanning. GitHub code scanning will flag any potential security issues in pull requests and on the repository’s security tab. Once that’s all working as it should, you can disable the LGTM.com integration.

@AdamRJensen
Copy link
Member Author

As described above, a PR has been made by GitHub #1600.

@kandersolar
Copy link
Member

I'm not convinced that LGTM/GitHub code scanning is particularly useful for us. Security vulnerability analysis doesn't seem relevant for pvlib (to me, anyway), and I think its other functionality (linting, static analysis) is redundant with our other CI checks. I'm also not excited about curating the list of "dismissed alerts". If it were me I would probably get rid of it for this repository. But I also haven't looked into it very much and would be happy to learn I am overlooking some nice aspects :)

@AdamRJensen
Copy link
Member Author

AdamRJensen commented Jan 17, 2023

Just as a comment, many of the issues that were flagged by LGTM were recently fixed in #1559. They were all rather minor, but nice to fix anyhow.

@wholmgren
Copy link
Member

For reference #554 added the LGTM badges. I liked them at the time because they showed that pvlib's code quality was very good and it did find a couple of minor bugs. I've been less than enthusiastic about it since then. I don't have a preference on this, just wanted to point to that context.

@AdamRJensen
Copy link
Member Author

Closing the PR and issue related to adding GitHub code scanning due to lack of interest from maintainers.

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

No branches or pull requests

3 participants