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

Solving lint rule issues #1403

Open
kinow opened this issue Aug 27, 2024 · 6 comments · May be fixed by #2040
Open

Solving lint rule issues #1403

kinow opened this issue Aug 27, 2024 · 6 comments · May be fixed by #2040
Assignees
Labels
discussion The issue is created to keep track a discussion

Comments

@kinow
Copy link
Member

kinow commented Aug 27, 2024

In GitLab by @LuiggiTenorioK on Aug 27, 2024, 14:49

The current source code of Autosubmit has many underlying issues that can be easily detected by linters.

Just running Ruff, we have the following rules that are violated:

$ ruff check --statistics autosubmit
191     F841    [*] unused-variable
 50     F401    [*] unused-import
 39     E721    [ ] type-comparison
 37     E402    [ ] module-import-not-at-top-of-file
 31     E722    [ ] bare-except
 18     F523    [ ] string-dot-format-extra-positional-arguments
 15     F541    [*] f-string-missing-placeholders
 15     F811    [ ] redefined-while-unused
 12     F901    [*] raise-not-implemented
  9     F821    [ ] undefined-name
  2     E712    [*] true-false-comparison
  2     F402    [ ] import-shadowed-by-loop-var
  1     E713    [*] not-in-test
  1     E731    [*] lambda-assignment

It will be nice to solve them to prevent future bugs and keep tracking them in future releases as part of our development process.

@kinow @dbeltrankyl @edgano

@kinow
Copy link
Member Author

kinow commented Aug 27, 2024

The current source code of Autosubmit has many underlying issues that can be easily detected by linters.

It does. If you ever open autosubmit.py, with its over 5K or 6K lines, it's simply impossible to read the code with the IDE without ignoring the warnings (I have to manually disable code inspection for that file).

It will be nice to solve them to prevent future bugs and keep tracking them in future releases as part of our development process.

+1, and have CICD to check for those, like the Workflow team is doing. @mcastril something that could be included for the September extraordinary team meeting agenda, IMO. We don't need to fix all at one, but maybe during August or December, when things are calmer, we could fix a bunch, and then the next chance we have we fix a little more...

Thanks Luiggi!

@kinow
Copy link
Member Author

kinow commented Aug 28, 2024

In GitLab by @dbeltrankyl on Aug 28, 2024, 10:30

Thanks Luiggi,

The IDE (pycharm) can solve a few of these issues with some inbuilt tools, I've used it a couple of times. I usually don't use it in a merge request because it makes modifications to all code and complicates the code review.

Maybe we should start by using that tool in a separate merge request, merge it, and we can include a procedure to run it in each new merge requests as it will only fix the new code added and doesn't complicate the code review

@kinow
Copy link
Member Author

kinow commented Aug 28, 2024

In GitLab by @dbeltrankyl on Aug 28, 2024, 15:15

mentioned in merge request autosubmit4-config-parser!9

@kinow
Copy link
Member Author

kinow commented Aug 28, 2024

In GitLab by @LuiggiTenorioK on Aug 28, 2024, 15:41

Maybe we should start by using that tool in a separate merge request, merge it, and we can include a procedure to run it in each new merge requests as it will only fix the new code added and doesn't complicate the code review

+1

The IDE (pycharm) can solve a few of these issues with some inbuilt tools, I've used it a couple of times. I usually don't use it in a merge request because it makes modifications to all code and complicates the code review.

Indeed, Ruff also can fix some of them "safely". I prefer to use Ruff since it also includes the code formatter and rules from other linters. Also, it is used in other projects in ES (like the one presented by Bruno and Aina) and works on different IDEs (VSCode and Pycharm as well).

@kinow
Copy link
Member Author

kinow commented Sep 2, 2024

In GitLab by @LuiggiTenorioK on Sep 2, 2024, 14:12

mentioned in issue autosubmit-api#87

@LuiggiTenorioK LuiggiTenorioK linked a pull request Dec 18, 2024 that will close this issue
@kinow
Copy link
Member Author

kinow commented Dec 19, 2024

This is something we can enlist Erick to help with. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion The issue is created to keep track a discussion
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants