Fix CodeQL warnings #740
Fix CodeQL warnings #740
Conversation
|
No rush for reviewing here. |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #740 +/- ##
==========================================
+ Coverage 92.85% 92.93% +0.08%
==========================================
Files 24 24
Lines 3329 3326 -3
Branches 583 579 -4
==========================================
Hits 3091 3091
Misses 171 171
+ Partials 67 64 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| return DnfTransactionHandler() | ||
| from convert2rhel.pkgmanager.handlers.dnf import DnfTransactionHandler | ||
|
|
||
| return DnfTransactionHandler() |
There was a problem hiding this comment.
Just a note, since you were asking about how to rewrite code that requires either dnf or yum in one of our 1::1s. This can be done at the toplevel with something like:
try:
from yum import *
# [...]
from convert2rhel.pkgmanager.handlers.yum import YumTransactionHandler as TransactionHandler
except ImportError:
from convert2rhel.pkgmanager.handlers.dnf import DnfTransactionHandler as TransactionHandlerThe naming (`TransactionHandler vs DnTransactionHandler) could be done inside of the specific handlers instead of in this file as well.
(This should not be done in this PR. Just seeing this reminded me of our discussion.)
There was a problem hiding this comment.
I think that it would cause a recursive import, as the {Yum,Dnf}TransactionHandler import from pkgmanager to do a lot of operations.
But once we have figured up a good way of transitioning to the refactor of yum/dnf, we can take care of that.
| stream.write("".join(manpage)) | ||
| stream.close() | ||
| with open(self.output, mode="w") as stream: | ||
| stream.write("".join(manpage)) |
There was a problem hiding this comment.
This build script is just copied into our repository from https://github.com/andialbrecht/build_manpage In my pylint cleanup patch, I excluded this file instead of fixing it in our copy so that we didn't get out of sync. OTOH, the upstream looks dead so maybe changing it is the right thing to do...
(No changes needed, just something to think about).
There was a problem hiding this comment.
I'd not be worried about diverging since as you mentioned the upstream is not maintained anymore.
r0x0d
force-pushed
the
fix-codeql-messages
branch
from
8621e90
to
e163cda
Compare
Done. |
This commit introduces a bunch of fixes for the warnings/notices that CodeQL was showing for Convert2RHEL under the the `Security` tab. Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>
Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>
r0x0d
force-pushed
the
fix-codeql-messages
branch
from
e163cda
to
9c81328
Compare
|
Rebased to trigger the tests once more. Probably if they fail for some reason will not be related to the PR itself, but just in case, triggering this again. |
|
Failures do not seem to be related to the changes made here. |
|
Merging this as it seems that the failures are not related to the PR, but rather some infrastructure problems. |
Venefilyn
added
kind/security
* Fix CodeQL warnings This commit introduces a bunch of fixes for the warnings/notices that CodeQL was showing for Convert2RHEL under the the `Security` tab. Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com> * Fix leftover warning from #723 Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com> --------- Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>
This commit introduces a bunch of fixes for the warnings/notices that CodeQL was showing for Convert2RHEL under the the
Securitytab.Jira Issue: RHELC-
Checklist
[RHELC-]is part of the PR titleRelease Pending