python-ipmi icon indicating copy to clipboard operation
python-ipmi copied to clipboard

Added Handeling for 'target is None' ipmitool.py

Open AliSot2000 opened this issue 3 years ago • 2 comments

Revised Pull Request of the previous pull request. The Codacy Complaint should be addressed now.

AliSot2000 avatar Jul 19 '22 19:07 AliSot2000

It would be great if you could meld this 2 commits into 1 commit.

https://stackoverflow.com/questions/2563632/how-can-i-merge-two-commits-into-one-if-i-already-started-rebase#:~:text=If%20you%20want%20to%20merge,commit%20name%20HEAD~2).&text=That%20will%20bring%20in%20the,them%20and%20you're%20done.

Then do a force push to your branch that will automatically update this PR. So you do not need to close and reopen a new PR.

Another thing is normally to add a "Signed-off-by" tag. It is optional but would also be good.

https://stackoverflow.com/questions/1962094/what-is-the-sign-off-feature-in-git-for

The last thing: An unittest would be great.

https://github.com/kontron/python-ipmi/blob/master/tests/interfaces/test_ipmitool.py#L23

Thanks

hthiery avatar Jul 20 '22 05:07 hthiery

Sorry for the wait. I've merged the commits (and the one for the unittest, which I added, as well), signed off the whole thing and force pushed the commit.

I hope it is up to spec.

AliSot2000 avatar Aug 14 '22 19:08 AliSot2000