Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add button to insert marker for unblocking incident to OSD branding #4867

Merged
merged 2 commits into from Oct 26, 2022

Conversation

Martchus
Copy link
Contributor

Copy link
Member

@okurz okurz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Can we see a screenshot for that and a live session demo-ing? Is it possible to add some more help text somewhere?

@Martchus
Copy link
Contributor Author

For everyone too lazy to checkout the branch locally:
grafik

Making the help popover extendable (for more help text) would likely be a bit hacky.

@okurz
Copy link
Member

okurz commented Oct 25, 2022

Making the help popover extendable (for more help text) would likely be a bit hacky.

Sure. I am just afraid that the text "Add marker to unblock incident" won't be enough for some users to understand that. Maybe just extend that line a bit. I know that providing a link would be tricky because people can't click on the hovering text.

@Martchus
Copy link
Contributor Author

Extended the tooltip a little bit.

@kraih
Copy link
Member

kraih commented Oct 25, 2022

Do we not have test coverage for those buttons yet?

@Martchus
Copy link
Contributor Author

For the buttons in general, yes. (But not for all of them as they're just using the same JS function anyways.)

@@ -0,0 +1,5 @@
% if (!$group_comment && !$job->is_ok) {
<a class="help_popover fa fa-unlink" title="Add marker so the specified maintenance incident will be approved despite this job"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment is not really correct because if other tests still fail the maintenance incident won't be approved but fine for now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the "despite this job" would be sufficient.

@mergify mergify bot merged commit 0e5d897 into os-autoinst:master Oct 26, 2022
@Martchus Martchus deleted the custom-commenting-tools branch October 26, 2022 20:08
os-autoinst-bot pushed a commit to os-autoinst-bot/openQA that referenced this pull request Oct 27, 2022
commit 0e5d897
Merge: b8fb43f c6c4b22
Author:     mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
AuthorDate: Wed Oct 26 19:18:09 2022 +0000
Commit:     GitHub <noreply@github.com>
CommitDate: Wed Oct 26 19:18:09 2022 +0000

    Merge pull request os-autoinst#4867 from Martchus/custom-commenting-tools

    Add button to insert marker for unblocking incident to OSD branding
@okurz
Copy link
Member

okurz commented Oct 27, 2022

Do we not have test coverage for those buttons yet?

Learning from https://progress.opensuse.org/issues/119467 it turns out we should have been a bit more persistent regarding test coverage ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants