-
Notifications
You must be signed in to change notification settings - Fork 124
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
Change result note key from a string to a list of strings #3254
base: main
Are you sure you want to change the base?
Conversation
@psss @thrix @lukaszachy This is definitely a breaking change for TF, and potentially for other users as well. TF ingests the field and treats it as a string, it will require at least two patches to accommodate the change - simple ones, but still. Yet I believe this change should happen because it better reflects the nature of the So, how to proceed to make it as painless for the end user as possible? Do we know who out there consumes |
713c0e6
to
f15eee5
Compare
I am fine introducing the breaking change, makes sense to me. I am not aware of any other user of I would like to take this as an example how the upcoming integration test to Testing Farm will provide a value here. Use it as a guinea pig showing that our Testing Farm worker code it is ready for this change. |
Still working it out, got blocked a bit :( |
@@ -266,7 +266,7 @@ Context: | |||
{% endfor %} | |||
</td> | |||
<td class="data"><a href="{{ base_dir | linkable_path | urlencode }}/{{ result.data_path | urlencode }}">data</a></td> | |||
<td class="note">{% if result.note %}{{ result.note | e }}{% else %}-{% endif %}</td> | |||
<td class="note">{% if result.note %}{{ result.printable_note | e }}{% else %}-{% endif %}</td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is printable_note
property actually needed? I think we should maybe print the notes as HTML list and keep the display/design of these notes on the Jinja template, e.g.:
{% if result.note %}
<ul>
{% for note in result.note %}
<li>{{ note | e }}</li>
{% endfor %}
</ul>
{% else %}
-
{% endif %}
f15eee5
to
0c8e436
Compare
This better represents its real nature: a collection of various notes, there may be more than one note or topic tmt needed to share with user depending on what happened while the test was running.
0c8e436
to
084e24a
Compare
This better represents its real nature: a collection of various notes, there may be more than one note or topic tmt needed to share with user depending on what happened while the test was running.
Pull Request Checklist