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

Pylint alerts corrections as part of an intervention experiment #306

Closed
evidencebp opened this issue Nov 9, 2024 · 6 comments
Closed

Pylint alerts corrections as part of an intervention experiment #306

evidencebp opened this issue Nov 9, 2024 · 6 comments

Comments

@evidencebp
Copy link
Contributor

Is your feature request related to a problem? Please describe.
The feature is not related to a problem but to code readability.

Describe the solution you'd like
I'd like to conduct a software engineering experiment regarding the benefit of Pylint alerts removal.
The experiment is described here.
In the experiments, Pylint is used with some specific alerts, files are selected for intervention and control.
After the interventions are done, one can wait and examine the results.

I'm asking for your approval for conducting an intervention in your repository.

See examples of interventions in stanford-oval/storm, gabfl/vault, and coreruleset/coreruleset.

You can see the planed interventions

May I do the interventions, @AndreiDrang ?

Describe alternatives you've considered
There is a single broad-exception-caught alert and the rest are too long lines.
This is not critical so an alternative will be to leave it as is.
However, the PR will be easy and there will be beneit in it.

Additional context

@AndreiDrang
Copy link
Owner

Hi @evidencebp, lets try it.

@evidencebp
Copy link
Contributor Author

I created a PR withthe changes.

Currently all of them are too long lines so rather minor.
I noted similarity in the file and I guess that some of the files in the control group have similar structure.
@AndreiDrang, can you please not delieretly fix them in the next few months?
That will be a great oppertunity to see empirically the impace of the change on close ojects.

@evidencebp
Copy link
Contributor Author

@AndreiDrang , I would like to consult you regarding the broad-exception-caught alert in src\python_rucaptcha\core\result_handler.py

Too braod exception might lead to problem if the developer planed to catch a specific exception and the exception also catchs and hides different exceptions.

Both get_sync_result (in line 34) and get_async_result (in line 63) catch Exception.
I could not understand which specific exceptions should e handles there.
Can you guide me? Thanks!

@evidencebp
Copy link
Contributor Author

@AndreiDrang , I got an alert that the tests fail.

I noted that the description is usually:
'errorDescription': 'The API key is missing in your request or has incorrect '
'format.',

My PR made long lines shorter.
Can you help me with that?

@AndreiDrang
Copy link
Owner

Hi
Thanks for your edits and sorry for the long silence.
I will accept your pull request today.
Regarding catching the error you wrote about above - I know this problem, but the code was added because the user doesn't care so much about the type of error if it occurs when receiving the result. Perhaps I'll change this if there are requests from users after the refactoring is finished.

@evidencebp
Copy link
Contributor Author

Thanks, @AndreiDrang!

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

No branches or pull requests

2 participants