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.

Sign up for GitHub

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

Jump to bottom

Add LGTM.com / DeepSource.io configuration files #909

Merged
merged 2 commits into from
Dec 17, 2021
Merged

Add LGTM.com / DeepSource.io configuration files #909

merged 2 commits into from
Dec 17, 2021

Conversation

DimitriPapadopoulos
Copy link
Contributor

The YAML configuration file for the LGTM static analysis tool can be either lgtm.yml or .lgtm.yml:
https://help.semmle.com/lgtm-enterprise/user/help/lgtm.yml-configuration-file.html

There is no need to integrate the LGTM tool in CI, as LGTM appears to be running on all repositories it has been run on once. The results currently appear here:
https://lgtm.com/projects/g/zarr-developers/zarr-python

The configuration file does not currently try to shut up any of the alerts:

See also #902 (comment).

TODO:

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions
  • New/modified features documented in docs/tutorial.rst
  • Changes documented in docs/release.rst
  • GitHub Actions have all passed
  • Test coverage is 100% (Codecov passes)

@DimitriPapadopoulos
Add LGTM.com configuration file
24d58cc 
The YAML configuration file for the LGTM static analysis tool can be
either lgtm.yml or .lgtm.yml:
  https://help.semmle.com/lgtm-enterprise/user/help/lgtm.yml-configuration-file.html

There is no need to integrate the LGTM tool in CI, as LGTM appears to
be running on all repositories it has been run on once. The results
currently appear here:
  https://lgtm.com/projects/g/zarr-developers/zarr-python
@DimitriPapadopoulos DimitriPapadopoulos marked this pull request as draft December 13, 2021 20:49
@DimitriPapadopoulos DimitriPapadopoulos mentioned this pull request Dec 13, 2021
Closed
@DimitriPapadopoulos
Add DeepSource.io configuration file
0036170 
The TOML configuration file is .deepsource.toml:
  https://deepsource.io/docs/concepts/#deepsourcetoml-file

DeepSource.io analysis must be enabled by a repository owner.

It doesn't look like it is possible to run DeepSource.io on each PR,
rather the monitored branch is analysed periodically, after commits
are pushed.
@DimitriPapadopoulos DimitriPapadopoulos changed the title Add LGTM.com configuration file Add LGTM.com / DeepSource.io configuration files Dec 13, 2021
@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented Dec 13, 2021 •
edited
Loading

The TOML configuration file for the DeepSource.io static analysis platform is .deepsource.toml:
https://deepsource.io/docs/concepts/#deepsourcetoml-file

As far as I can see, it is not possible to silence a class of alerts in the configuration file. Repository owners need to silence each alert individually from the user interface:
https://deepsource.io/blog/releases-issue-actions/

@DimitriPapadopoulos DimitriPapadopoulos marked this pull request as ready for review December 13, 2021 22:21
joshmoore
joshmoore approved these changes Dec 14, 2021
View reviewed changes
Copy link
Member

@joshmoore joshmoore left a comment

Choose a reason for hiding this comment

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

LGTM.

Only other thought that comes to mind is whether or not to add README badges.

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented Dec 14, 2021 •
edited
Loading

It's perhaps a bit early for the badges:

  • DeepSource.io should be enabled on the repository first,
  • LGTM.com could be enabled on PRs,
  • a few more issues could be fixed first,
  • we cannot get rid of all false positives or questionable alerts (especially with DeepSource.io).

@jeromekelleher jeromekelleher mentioned this pull request Dec 15, 2021
Closed
@joshmoore joshmoore merged commit 4fb0f7e into zarr-developers:master Dec 17, 2021
@DimitriPapadopoulos DimitriPapadopoulos mentioned this pull request Jul 15, 2022
Merged
7 tasks
@DimitriPapadopoulos
Copy link
Contributor Author

Because Semmle has joined GitHub, LGTM.com will be deprecated and replaced by GitHub code scanning.

The next step for LGTM.com: GitHub code scanning!

As far as I can understand, in simple cases such as this one, automated pull requests will be created to help us migrate:

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.

joshmoore added a commit that referenced this pull request Sep 8, 2022
@joshmoore
Create codeql-analysis.yml
e388728 
see:
 - https://github.blog/2022-08-15-the-next-step-for-lgtm-com-github-code-scanning/
 - #909 (comment)
@joshmoore joshmoore mentioned this pull request Sep 8, 2022
Merged
6 tasks
@joshmoore
Copy link
Member

Thanks for keeping us uptodate, @DimitriPapadopoulos. See #1127

joshmoore added a commit that referenced this pull request Sep 22, 2022
@joshmoore
Create codeql-analysis.yml (#1127)
3632820 
* Create codeql-analysis.yml

see:
 - https://github.blog/2022-08-15-the-next-step-for-lgtm-com-github-code-scanning/
 - #909 (comment)

* Re-implement tempfile.mktemp using NamedTemporaryFile

Adds zarr.tests.util.mktemp which can be used from all
tests. The NamedTemporaryFile is immediately closed and
only the path returned.
@DimitriPapadopoulos DimitriPapadopoulos mentioned this pull request Oct 15, 2022
Merged
6 tasks
enthusiastdev121 added a commit to enthusiastdev121/zarr-python that referenced this pull request Aug 19, 2024
@enthusiastdev121
Create codeql-analysis.yml (#1127)
b477397 
* Create codeql-analysis.yml

see:
 - https://github.blog/2022-08-15-the-next-step-for-lgtm-com-github-code-scanning/
 - zarr-developers/zarr-python#909 (comment)

* Re-implement tempfile.mktemp using NamedTemporaryFile

Adds zarr.tests.util.mktemp which can be used from all
tests. The NamedTemporaryFile is immediately closed and
only the path returned.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joshmoore joshmoore joshmoore approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DimitriPapadopoulos @joshmoore