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

Create .codeclimate.yml #84

Merged
merged 2 commits into from
Jul 26, 2021
Merged

Create .codeclimate.yml #84

merged 2 commits into from
Jul 26, 2021

Conversation

BoxiLi
Copy link
Member

@BoxiLi BoxiLi commented Jul 25, 2021

Copy the code climate settings from qutip

Copy the code climate settings from qutip
@BoxiLi BoxiLi requested a review from hodgestar July 26, 2021 11:23
@BoxiLi
Copy link
Member Author

BoxiLi commented Jul 26, 2021

As this is the initial commit of codeclimate, I think we can merge this as-is and solve those climate issues later step by step?

.codeclimate.yml Outdated
exclude_patterns:
- "doc/"
- "dist/"
- "**/tests/"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we include tests in CodeClimate? Tests are code too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I agree.

@hodgestar
Copy link
Contributor

I would personally prefer not to use a tool run by a third party like CodeClimate and rather to just run these tools ourselves in CI (e.g. flake8), but qutip itself already uses CodeClimate and it's better to have something than nothing, so let's go for it.

Aside: Reasons for not liking a third party tool here -- it's a real pain to run CodeClimate checks locally and CodeClimate maintains state about which "bad" bits of code should be ignored that is completely outside of our control and invisible in the repository.

Copy link
Contributor

@hodgestar hodgestar left a comment

Choose a reason for hiding this comment

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

I left a question about using CodeClimate on tests and some thoughts about CodeClimate itself, but I am also +1 on merging this as is.

@BoxiLi
Copy link
Member Author

BoxiLi commented Jul 26, 2021

it's a real pain to run CodeClimate checks locally

That is true. I don't know a way to run it locally either. Codeclimate checks a few more things as flake8, like similar code block and too complicated logic etc. We could find something that works both for qutip and qutip-qip.

@BoxiLi
Copy link
Member Author

BoxiLi commented Jul 26, 2021

Interestingly, now you can choose in codeclimate to approve the issues for this PR so that the tests will all pass.

@hodgestar
Copy link
Contributor

That is true. I don't know a way to run it locally either. Codeclimate checks a few more things as flake8, like similar code block and too complicated logic etc. We could find something that works both for qutip and qutip-qip.

There is a radon plugin for flake8 that will check code complexity -- https://github.com/rubik/radon -- and which I usually use with flake8.

@BoxiLi BoxiLi merged commit 10c6db1 into master Jul 26, 2021
BoxiLi added a commit to BoxiLi/qutip-qip that referenced this pull request Aug 1, 2021
Copy the code climate settings from qutip but also apply code climate to tests
@BoxiLi BoxiLi deleted the code-climate branch November 25, 2021 08:39
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.

None yet

2 participants