search

milesmcc / shynet

Modern, privacy-friendly, and detailed web analytics that works without cookies or JS.
Apache License 2.0
2.89k stars 183 forks source link

Add tests for DashboardApiView and ApiTokenRequiredMixin #230

Closed sumit4613 closed 11 months ago

sumit4613 commented 1 year ago

167

100% coverage for api app. image

haaavk commented 1 year ago

Nice work with 100% code coverage. Usually it's not a best idea to mix refactoring with other changes in one commit. Adding pure subjective code style changes may not be worth it. Some of them are likely to be undone next time I'll work on API.

sumit4613 commented 1 year ago

Hey, @haaavk understood, I'll move tests and refactoring to separate commits.

sumit4613 commented 1 year ago

Hey, @haaavk I have separated my changes into different commits. Please review.

Also, I'm hoping you guys don't have any issues using black for code formatting.

haaavk commented 1 year ago

Black is cool. I personally prefer to keep lines length under 100. @milesmcc Are there any official code style rules for Shynet?

sumit4613 commented 1 year ago

Black is cool

✅ 🙌🏽

I personally prefer to keep lines length under 100.

I see, I keep them around 120.

Are there any official code style rules for Shynet?

It'll be great if we can have official code style rules or a .editorconfig file.

milesmcc commented 1 year ago

Yeah @haaavk Black is what I prefer. We should codify this.

sumit4613 commented 1 year ago

@milesmcc Got any spare time to review this as well? 🤔