Change softfailure to info for s390x shutdown

[rakoenig]

Backend does not implement is_shutdown so there is not much chance to detect a shutdown from the GUI. Nevertheless we can replace the softfailure with just an info.

• Related ticket: https://progress.opensuse.org/issues/110437
• Needles: -
• Verification run: https://openqa.suse.de/tests/9072280#

[github-actions]

Great PR! Please pay attention to the following items before merging:

Files matching lib/**.pm:

• Consider adding or extending unit tests in t/

This is an automatically generated QA checklist based on modified files

[okurz]

The commit message could have a bit more details, e.g. what issue is fixed, why this needs to change, to which versions of which product it applies, link to a bug or a fate entry, the choices you made, etc. Also see https://commit.style/ or http://chris.beams.io/posts/git-commit/ as a helpful guide how to write good commit messages.

I recommend for the subject line to tell about the important effect, not just what you changed.

What you put into the description of the pull request could also be in the commit message details. I don't know how you created your pull request but your git commit messages has only the subject line while the pull request description has more details. Please keep in mind that the github pull request description is only visible on github, the commit can be considered permanent information storage.

I can recommend the tool hub (zypper in rubygem-hub) for easier PR creation. Also, I myself use a script git-pr-last to create a PR with proper description for these simple one-commit PRs:

$ cat $(which git-pr-last )
#!/bin/sh -e
target="${target:-"$USER"}"
git push $target && git show --no-patch --format=%B | hub pull-request -F -

See https://github.com/os-autoinst/os-autoinst-distri-opensuse/blob/master/CONTRIBUTING.md#coding-style for more details

Also the change of newline at the end of the file is an unrelated whitespace change that should be avoided within the same commit. You can create a separate commit in the same PR though.

[ge0r]

LGTM!

[ge0r]

Maybe for better visibility you could do
record_info('poo#58127', 'Temporary workaround, because shutdown module is marked as failed on s390x backend when shutting down from GUI.');

[okurz]

@ge0r have you overlooked my review or did you think it was not important or already resolved (which it wasn't)?

[ge0r]

@ge0r have you overlooked my review or did you think it was not important or already resolved (which it wasn't)?

I did not overlook it, which is why I did not merge the PR, it did not make sense for me to repeat the commit message point since it was already addressed, so I marked it as ok as far as the implementation went, with a suggestion from my side.

[okurz]

But my review included multiple points and only one was addressed. I would prefer if a review is taken seriously and not disregarded, especially not without a comment, e.g. "sorry, I need to merge this because whole production is down today and we urgently need this fixed and the git commit author has gone to vacation and nobody else can fix the typo in the comment that you flagged".

[jknphy]

Hi @rakoenig as we agreed a time ago, after our initial investigation of the issue by a couple of team members in our squad we didn't find what was the problem, so we will be creating the record_info with a ticket for the tools teams's backlog. We should not reference the progress ticket 'Resolved', which by they way we use 'Close' in our flow.

Other than that, regarding the feedback, I didn't see the previous version of the PR, but now looks sort of ok, I would only had added 'shutdown from GUI' to title/commit to make it more clear probably. It is a good suggestion from Oliver to use some automatic tool, as I can see that we sometimes forget to match PR title and PR commit, please consider that.