removed '@typescript-eslint/no-shadow': 'off' rule

[Aakarshan-369]

removing the second instance of no-shadow: 'off' rule as mentioned in #2089

[welcome[bot]]

Welcome to the SigNoz community! Thank you for your first pull request and making this project better. 🤗

[Aakarshan-369]

@palashgdev I removed the @typescript-eslint/no-shadow': 'off' rule and I am getting a lot of error 'event' is already declared in the upper scope error from the no-shadow checks after I run the run lint command in the dev env.
I'm checking up on the no-shadow documentation. Any other way to resolve this? Thanks!

[palashgdev]

Maybe changing the name ?

[Aakarshan-369]

@palashgdev I checked and the no-shadow issue is present in multiple files (caused by same-name variables). Just like you suggested I think the solution is to rename all instances of locally scoped variables to not match the global one. I think the files which need this change were throwing the scope error in the Run ESLint of the build-frontend process I hope I'm on the right track here. Thanks!

[palashgdev]

Yup you are on the right track ! but would be good to have a right and meaningful name...

[Aakarshan-369]

Hi @palashgdev It took some time but I managed to eliminate the @typescript-eslint/no-shadow': 'off' I essentially renamed all the local instances of variables not to match the global one. I ran npm run lint and got no errors in my dev environment. I tried building using yarn dev and it seems to work fine.

The Quality Gate test is failing and I'm not sure how to resolve it, I would really appreciate any feedback and further changes needed for this to work :)

Thanks so much! I'm learning so much from the codebase :) 👍

[palashgdev]

Right now we are not looking into sonar analysis you can skip that one but in future we will

[Aakarshan-369]

Thanks for the review! Understood the sonar check part.

Anything else that needs work in this PR?

One thing I noticed while working on this was that the linebreak rule in elintjs is not working properly in detecting my windows os (it threw around 40k linebreak errors). On my linux env it wasn't a problem.

[palashgdev]

Sure there are some place in which we can give better name will review after some time

[makeavish]

@Aakarshan-369 Please resolve merge conflicts

[Aakarshan-369]

@makeavish working on it 👍

[palashgdev]

@Aakarshan-369 I tried to review your PR and offered some suggestions for variable names. If you think we can do better, we can talk about that.

[Aakarshan-369]

@palashgdev Thanks for the Review! After correcting all the ESlint no-shadow issue I was trying to resolve this build error, https://github.com/SigNoz/signoz/actions/runs/4053750779/jobs/6974737091 I have reverted those changes and resolved a few merge conflicts. Now working on updating a few variable names again. Which files do you think renaming? I'm thinking eParam and treeParam and similar variables need better naming.

[Aakarshan-369]

Hi! @palashgdev thanks for the review and suggestions! I have made the changes as needed and as you suggested. I'm curious to know if more changes or updates are needed. Thanks :)

[palashgdev]

Hi! @palashgdev thanks for the review and suggestions! I have made the changes as needed and as you suggested. I'm curious to know if more changes or updates are needed. Thanks :)

Thanks @Aakarshan-369 let me go through it

[Aakarshan-369]

Hi @palashgdev sir, I have resolved a few merge conflicts and removed the newer lint errors. I am looking forward to knowing if more updates or changes need to be made. Thanks! :)

[palashgdev]

Also @Aakarshan-369 please resolve the conservation which are resolved it is easier to track

[github-advanced-security[bot]]

You have successfully added a new SonarCloud configuration ``. As part of the setup process, we have scanned this repository and found no existing alerts. In the future, you will see all code scanning alerts on the repository Security tab.

[Aakarshan-369]

@palashgdev Apologies sir for the long hiatus, I've left comments in the code review and would love to know what further work is needed. Thanks!

[sonarcloud[bot]]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
5.0% Duplication

[Aakarshan-369]

Hi @palashgdev sir, should I make the necessary changes to the variable names as mentioned by me in the code review above? Maybe that would resolve all the issues. Thanks!

[CLAassistant]

All committers have signed the CLA.