improvement(treewide): Remove pylint

[pehala]

Pylint is no longer used, I think we can remove all the pylint disables. Ruff has less checks than pylint, if needed we can add more check Ruff to compensate (followup PR).

Warning: This might negatively affect backports, if we are backporting any line that was disabled, but I think that is small risk

Testing

• None needed

PR pre-checks (self review)

• I added the relevant backport labels
• I didn't leave commented-out/debugging code

Reminders

• Add New configuration option and document them (in sdcm/sct_config.py)
• Add unit tests to cover my changes (under unit-test/ folder)
• Update the Readme/doc folder relevant to this change (if needed)

[fruch]

It's only v15 that has pylint...

So I think we are good to go

[fruch]

LGTM

[fruch]

If we won't backport it, it's gonna mess up any backport to branches

so I suggest backporting it to all branches

[soyacz]

If we won't backport it, it's gonna mess up any backport to branches

so I suggest backporting it to all branches

did we backported ruff to all supported branches (e.g. perf-v15?)

[pehala]

did we backported ruff to all supported branches (e.g. perf-v15?)

Based on what @fruch said, all except for this branch should support it.

so I suggest backporting it to all branches

I added some, but I am not sure if I added correct ones, please correct them if you think they are wrong

[pehala]

tests failed on jenkins issue

[vponomaryov]

Better to make such huge updates that not really dependent on each other in separate PRs.
As of now it is not backportable, it is easy to get lost in the list of changes.

I would like to see the changes be split into batches, for example - by 10 modules.

[pehala]

Better to make such huge updates that not really dependent on each other in separate PRs. As of now it is not backportable, it is easy to get lost in the list of changes.

I would like to see the changes be split into batches, for example - by 10 modules.

I do not see logic in splitint by certain number of modules, it will generate too many needless PR (i.e. even more spam), the change is consise and even though it is big all changes relate to only this. I am fine with manually fixing the backport branches, it will be manual but not hard, I do not think it will take me more than 15 mins to fix a backport PR.

[vponomaryov]

Better to make such huge updates that not really dependent on each other in separate PRs. As of now it is not backportable, it is easy to get lost in the list of changes.
I would like to see the changes be split into batches, for example - by 10 modules.

I do not see logic in splitint by certain number of modules, it will generate too many needless PR (i.e. even more spam), the change is consise and even though it is big all changes relate to only this. I am fine with manually fixing the backport branches, it will be manual but not hard, I do not think it will take me more than 15 mins to fix a backport PR.

5 hours have passed since your latest push and it already has merge conflict.
So, it is first reason. Changing half of the code base in a single PR must have serious reasons which are absent here.
We can remove pylint comments even 1 by 1. We are not even dependent on it's removal.

Another reason - possibility to get human reviews.
Who is going to review 2500 lines of code changes for stuff which get merge conflicts that soon?
And "just believe me it is ok and I didn't do any mistake removing some unexpected stuff" is not argument.

Doing smaller batches we make less problems.

[pehala]

Better to make such huge updates that not really dependent on each other in separate PRs. As of now it is not backportable, it is easy to get lost in the list of changes.
I would like to see the changes be split into batches, for example - by 10 modules.

I do not see logic in splitint by certain number of modules, it will generate too many needless PR (i.e. even more spam), the change is consise and even though it is big all changes relate to only this. I am fine with manually fixing the backport branches, it will be manual but not hard, I do not think it will take me more than 15 mins to fix a backport PR.

5 hours have passed since your latest push and it already has merge conflict. So, it is first reason. Changing half of the code base in a single PR must have serious reasons which are absent here. We can remove pylint comments even 1 by 1. We are not even dependent on it's removal.

There is no way of preventing merge conflicts, even if we split we will have merge conflicts, only in a fewer PR.
Splitting it into batches also creates PR spam, 10 batches creates 10x PR (around 60 for that matter), and it wont prevent merge conflicts either, I will just have to care fixing 5 PR instead of 1. This PR should not exist, because we never should have left the pylint disables in here for a year after removing the actual pylint.

Nothing is dependening on this PR and this PR is harmless as it lacks functional changes.

Another reason - possibility to get human reviews. Who is going to review 2500 lines of code changes for stuff which get merge conflicts that soon? And "just believe me it is ok and I didn't do any mistake removing some unexpected stuff" is not argument.

I doubt that review of this PR will take more than 10 minutes. It is pretty clear from the git diff showned in github that it indeed does not modify any code and thus there is no need to investigate impacts of each individual line. It is possible that I chnages something which is not that just removal of pylint disables, but even that would be easy to find as ANY change that is not easy to read from the diff is a mistake regardless of if it impacts anything and should be removed.

Doing smaller batches we make less problems.

Doing smaller batches will require (number of batches) times more attention and will take (number of batches) more time to complete with no clear benefit.

In my opinion, this PR can be approved and merge withing 20 mins, only thing that needs to be done is to synchronize (which would be my problem) 2 reviewers and one of them with merge rights and we can get it easily and noone has to care about it again, it might take a week or two before we synchronize but when we do I believe it is gonna be painless. Consequence of this is gonna be a one or two PR that will have conflcits from this but I do not see that as a problem (That would probably still happen even with smaller PRs). It is an equivalent of ripping the Band-Aid quickly vs ripping slowly, which would take weeks

If you really feel that strongly about this, give a final word and I will split it up to any desired number of batches

[soyacz]

I don't see the point of splitting.
If we merge it and backport swiftly then we won't see merge conflicts around this one - except perf v-15 branch, hopefully we could work towards its deprecation.

[vponomaryov]

There is no way of preventing merge conflicts, even if we split we will have merge conflicts, only in a fewer PR.

Right, creating 10 PRs you would get, for example, 1-2 merge conflicts in 5 hours getting easy +1 approve.
Then in 20 more hours, probably, 1-2 more merge conflicts and one more +1 approve.
As a result we merge 6 out 10 PRs and only 40% left to resolve merge conflicts and probabilistically we hit less merge conflicts in smaller parts.
Merge conflicts are ok, the question is in the re-review efforts.

Splitting it into batches also creates PR spam, 10 batches creates 10x PR (around 60 for that matter)

What is PR spam?
As more your PR may stay without merge conflicts as better for it's mergeability.
All of us are busy.

we never should have left the pylint disables in here for a year

The point is in the backports, only one branch left to be incompatible - branch-perf-v15, which is ok based on comments above.

Nothing is dependening on this PR and this PR is harmless as it lacks functional changes.

this PR is harmless - it is matter of belief while reviewers haven't finished their reviews.

Another reason - possibility to get human reviews. Who is going to review 2500 lines of code changes for stuff which get merge conflicts that soon? And "just believe me it is ok and I didn't do any mistake removing some unexpected stuff" is not argument.

I doubt that review of this PR will take more than 10 minutes. It is pretty clear from the git diff showned in github that it indeed does not modify any code and thus there is no need to investigate impacts of each individual line. It is possible that I chnages something which is not that just removal of pylint disables, but even that would be easy to find as ANY change that is not easy to read from the diff is a mistake regardless of if it impacts anything and should be removed.

10 minutes? Only partial review, I disagree that 10 minutes will be taken for complete review.

Doing smaller batches we make less problems.

Doing smaller batches will require (number of batches) times more attention and will take (number of batches) more time to complete with no clear benefit.

Your goal is to get code changes be merged, not minimizing the number of PRs.

In my opinion, this PR can be approved and merge withing 20 mins, only thing that needs to be done is to synchronize (which would be my problem) 2 reviewers and one of them with merge rights and we can get it easily and noone has to care about it again, it might take a week or two before we synchronize but when we do I believe it is gonna be painless. Consequence of this is gonna be a one or two PR that will have conflcits from this but I do not see that as a problem (That would probably still happen even with smaller PRs). It is an equivalent of ripping the Band-Aid quickly vs ripping slowly, which would take weeks

If you really feel that strongly about this, give a final word and I will split it up to any desired number of batches

Smaller viable changes is always a better option.
This point ^ is very important.

You have great ideas, great change descriptions but huge PRs, moreover, several your big PRs have merge incompatibilities among themselves - nemesis changes.
Where I still have no clarity about readiness, order of merge and all that stuff.
And, as a result, nothing gets merged.

So, if one has smaller viable changes as a priority then he gets more gain out of his efforts, IMHO.