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

[Enhancement]Fix code quality issues #1693

Closed
withshubh opened this issue Dec 22, 2020 · 7 comments
Closed

[Enhancement]Fix code quality issues #1693

withshubh opened this issue Dec 22, 2020 · 7 comments
Labels
good first issue A user wrote a good first issue with clear instructions 🤘 status:resolved 🏋️‍♀️ type:task

Comments

@withshubh
Copy link
Contributor

withshubh commented Dec 22, 2020

Hi 👋
I ran the DeepSource static analyzer on the forked copy of iris repo, and found some interesting code quality issues.

I am opening a PR to fix a few of them.

@withshubh withshubh changed the title Fix code quality issues [Enhancement]Fix code quality issues Dec 22, 2020
@kataras
Copy link
Owner

kataras commented Dec 22, 2020

Hello @withshubh,

Thanks for offering to help us out, please feel free to push a complete PR. Just one note of my behalf: please benchmark before changes in hot-paths, if the old code looks bad but it performs faster, keep it as it is. I am sure you will find interesting things, after all, it's a project of 24/24⌚ 5 years of hard work 💪

Thanks,
Gerasimos Maropoulos.

@kataras
Copy link
Owner

kataras commented Jan 9, 2021

Hello @withshubh do you want to continue this, can I leave it up for you or we should close this one? (Your first PR is merged, and thanks )

@kataras kataras added the good first issue A user wrote a good first issue with clear instructions label Jan 9, 2021
@withshubh
Copy link
Contributor Author

You can close this issue for now!
I may or may not submit the PR with new fixes but I'll submit a new PR to add a DeepSource badge to your GitHub readme which will help project contributors to fix the issues in the repo.

@kataras
Copy link
Owner

kataras commented Jan 9, 2021

@withshubh I've already tried to do that but it doesn't match the UI, we can't add more badges for now and we shouldn't, users who want to help they can just click the last commit, there they can find the DeepSource build, even in their PRs, so we are ok in that part.

Eventually I will fix all these issues alone I already know it (suddenly) :P And I was thinking.... do you have editor plugin? (i.e VS Code, which I can navigate through the issues and fix those in-time, like go-vet/language server does). I should read the documentation as well, some recommendation are not actual fixes, e.g. here, if we follow the recommendation the result template file will not be rendered as html, there should be an option for // lint: ignore type of thing somewhere.

And one more, you are free to put Iris in your users list 👍🏽

@kataras
Copy link
Owner

kataras commented Jan 9, 2021

Here it's: #1701, now we can close this issue :)

@withshubh
Copy link
Contributor Author

do you have editor plugin? (i.e VS Code, which I can navigate through the issues and fix those in-time, like go-vet/language server does).

Sublime and VSCode plugin will be available by the end of the March. 🎉

there should be an option for // lint: ignore type of thing somewhere.

You can use // skipcq at the end of that line or you can ignore issues directly from the DeepSource dashboard by clicking on Ignore this issue.

And one more, you are free to put Iris in your users list 👍🏽

Thanks @kataras 💖 On it!

@kataras kataras closed this as completed Jan 21, 2021
@kataras
Copy link
Owner

kataras commented Jan 21, 2021

Thanks @withshubh yes, false alarms were ignored through dashboard but skipcq seems a smarter choice :)

Of course, tell me if you need more from my side too.

I am closing this issue as all current reported mistakes were fixed 12 days ago.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue A user wrote a good first issue with clear instructions 🤘 status:resolved 🏋️‍♀️ type:task
Projects
None yet
Development

No branches or pull requests

2 participants