Rename hostRequires_list to host_requires_list #140
Conversation
Pull Request Test Coverage Report for Build 717
💛 - Coveralls |
|
Hi! This is the friendly CKI test bot. The maintainers can mention me in a comment together with the word "test" and I will test this PR and post the results. Please note that the only tests that will be tested by the bot are those that are enabled in kpet-db or are already in use within CKI pipelines. If your test is new and has not been enabled for use in pipelines, the bot cannot test it here. |
emanchado
force-pushed
the
FASTMOVING-1612-consistent-naming-conventions
branch
from
1ac4ace
to
3ab3ac6
Compare
|
This looks good, thank you! However, you gotta take care of transitioning kpet-db to the new name smoothly. So instead start with exposing both the old name and the new name, submit an MR to kpet-db, which switches over to the new name, then when that's merged, submit another one to kpet, which removes the old name. |
emanchado
force-pushed
the
FASTMOVING-1612-consistent-naming-conventions
branch
from
3ab3ac6
to
626cbad
Compare
|
Oops, my bad. I thought of it, but then wrongly thought that it was just internal and it wasn't a problem. Oh well. See the update. |
kpet/run.py
Outdated
| self.partitions_list = filter(lambda e: e is not None, | ||
| partitions_list) | ||
| self.kickstart_list = filter(lambda e: e is not None, | ||
| kickstart_list) | ||
| # For compatibility. Remove when kpet-db is updated. |
There was a problem hiding this comment.
Can you add TODO: in front of this comment? Then it will be easy to find.
kpet/run.py
Outdated
| self.partitions_list = filter(lambda e: e is not None, | ||
| partitions_list) | ||
| self.kickstart_list = filter(lambda e: e is not None, | ||
| kickstart_list) | ||
| # For compatibility. Remove when kpet-db is updated. | ||
| self.hostRequires_list = self.host_requires_list |
There was a problem hiding this comment.
Can you move this line, along with the comment to be together with the self.host_requires_list assignment?
Although it does hold a list of "hostRequires", it's a bit ugly that we end up with a variable that has both mixedCase and snake_case in the same name. So, default to the normal Python convention and use "host_requires_list" for the variable. However, need to keep hostRequires_list temporarily for compatibility reasons. Ref. FASTMOVING-1612
emanchado
force-pushed
the
FASTMOVING-1612-consistent-naming-conventions
branch
from
626cbad
to
1b86b16
Compare
|
@cki-bot: test! |
|
CKI pipeline triggered |
|
See #141 for the test results. |
|
#141 was merged, so no sense in keeping this anymore |
Although it does hold a list of "hostRequires", it's a bit ugly that
we end up with a variable that has both mixedCase and snake_case in
the same name. So, default to the normal Python convention and use
"host_requires_list" for the variable.
Ref. FASTMOVING-1612