shynet icon indicating copy to clipboard operation
shynet copied to clipboard

Add tests for DashboardApiView and ApiTokenRequiredMixin

Open sumit4613 opened this issue 3 years ago • 6 comments

#167

100% coverage for api app. image

sumit4613 avatar Sep 26 '22 04:09 sumit4613

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.

haaavk avatar Sep 26 '22 08:09 haaavk

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

sumit4613 avatar Sep 26 '22 08:09 sumit4613

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.

sumit4613 avatar Sep 26 '22 12:09 sumit4613

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

haaavk avatar Sep 26 '22 16:09 haaavk

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.

sumit4613 avatar Sep 26 '22 17:09 sumit4613

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

milesmcc avatar Sep 27 '22 04:09 milesmcc

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

sumit4613 avatar May 24 '23 19:05 sumit4613