#54437 closed defect (bug) (fixed)
Warning not in bold?
Reported by: | NekoJonez | Owned by: | audrasjb |
---|---|---|---|
Milestone: | 6.0 | Priority: | normal |
Severity: | trivial | Version: | |
Component: | General | Keywords: | good-first-bug has-patch has-unit-tests i18n-change |
Focuses: | ui, administration, template, coding-standards | Cc: |
Description (last modified by )
Most of the warnings in WP have the "warning" part in bold. Yet, there are a few that aren't in bold.
Except these:
https://build.trac.wordpress.org/browser/tags/5.9.2/wp-includes/functions.php?marks=1583#L1582
https://build.trac.wordpress.org/browser/tags/5.9.2/wp-includes/class-wp-theme.php?marks=284#L283
For the sake of being consistent, shouldn't these have bold tags as well?
Attachments (1)
Change History (33)
#3
in reply to:
↑ description
@
3 years ago
- Focuses ui template added
- Type changed from enhancement to defect (bug)
- Version changed from 5.8.2 to 5.8.3
In the latest dev builds, the locations are changed:
https://build.trac.wordpress.org/browser/trunk/wp-includes/functions.php?marks=1582#L1592
https://build.trac.wordpress.org/browser/trunk/wp-includes/class-wp-theme.php?marks=284#L285
#4
@
3 years ago
- Description modified (diff)
- Keywords needs-patch good-first-bug added
- Milestone changed from Awaiting Review to 6.0
- Version 5.8.3 deleted
Thanks for the ticket!
Updated the description with permanent links so that the locations do not change. Removing the version, as these messages were not introduced in 5.8.2 or 5.8.3.
It looks like most of the "Error:" prefixes in core are placed inside a <strong>
tag, so I think it makes sense to do the same here.
This ticket was mentioned in PR #2551 on WordPress/wordpress-develop by audrasjb.
3 years ago
#6
Trac ticket: https://core.trac.wordpress.org/ticket/54437
#7
@
3 years ago
- Keywords commit assigned-for-commit added
We reviewed this patch during WordCamp Geneva 2022’s contribution workshop.
The above PR was meant to test the provided patch.
Looks like tests are passing.
Self assigning for commit
.
#8
@
3 years ago
- Owner set to audrasjb
- Resolution set to fixed
- Status changed from new to closed
In 53118:
3 years ago
#9
Committed in https://core.trac.wordpress.org/changeset/53118
#10
@
3 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
I'm reopening this, since I found another one during translation.
https://build.trac.wordpress.org/browser/trunk/wp-admin/includes/ajax-actions.php?marks=1294#L1294
This ticket was mentioned in PR #2666 on WordPress/wordpress-develop by NekoJonez.
3 years ago
#11
During development of 6.0, a trac ticket of mine was fixed. This was adding strong tags in warnings. Yet, during translation I found two others.
Trac ticket: (https://core.trac.wordpress.org/ticket/54437)
This ticket was mentioned in Slack in #core by nekojonez. View the logs.
3 years ago
NekoJonez commented on PR #2666:
3 years ago
#13
I should have checked the other branches before submitting this PR... Now, I have found another one. All branches have been checked and I think I got them all.
#15
in reply to:
↑ 14
@
3 years ago
Replying to audrasjb:
Thanks @NekoJonez 👍
No problem @audrasjb sorry for not checking the other translation branches as well before first submitting the PR... Now, I got them all so I think if no objections in this PR happen it's all fine to go and get this ticket re-fixed :)
3 years ago
#16
@NekoJonez tests are now failing though:
1) Tests_Ajax_ReplytoComment::test_with_draft_post Failed asserting that exception message '<strong>Error:</strong> You cannot reply to a comment on a draft post.' contains 'Error: You cannot reply to a comment on a draft post.'.
You need to also update unit tests :)
NekoJonez commented on PR #2666:
3 years ago
#17
@NekoJonez tests are now failing though:
1) Tests_Ajax_ReplytoComment::test_with_draft_post Failed asserting that exception message '<strong>Error:</strong> You cannot reply to a comment on a draft post.' contains 'Error: You cannot reply to a comment on a draft post.'.You need to also update unit tests :)
As a noob in this, some help with that would be nice.
NekoJonez commented on PR #2666:
3 years ago
#18
@ocean90 , I removed all strong tags in the AJAX php file... And tests are still failing.
Do those strong tags also need to be removed in the other files?
This ticket was mentioned in PR #2669 on WordPress/wordpress-develop by audrasjb.
3 years ago
#19
- Keywords has-unit-tests added
Trac ticket: https://core.trac.wordpress.org/ticket/54437
3 years ago
#20
@NekoJonez I opened another PR to fix the failing unit tests: https://github.com/WordPress/wordpress-develop/pull/2669
Does it look good to you?
NekoJonez commented on PR #2666:
3 years ago
#21
@NekoJonez I opened another PR to fix the failing unit tests: https://github.com/WordPress/wordpress-develop/pull/2669
Does it look good to you?
@audrasjb Excellent. Now I dont have to worry about why my things are failing.
3 years ago
#22
Closing in favor of https://github.com/WordPress/wordpress-develop/pull/2669
NekoJonez commented on PR #2669:
3 years ago
#23
Excellent! Thank you @audrasjb for this help. I have no idea why for me the unit tests were failing.
3 years ago
#25
Committed in https://core.trac.wordpress.org/changeset/53337
#26
@
3 years ago
We're currently in soft string freeze as per the 6.0 release schedule, so no strings should be changed at this point, except for the About page. I have no strong objections to leaving this change in 6.0, but as these strings don't seem new, I think it would have been better to handle this in a separate ticket early in 6.1 instead, same as #55661.
#27
@
3 years ago
- Keywords assigned-for-commit removed
@SergeyBiryukov yes, but it appears this issue was spotted while translating 6.0 strings, so in my understanding – and I might be wrong :) – the current phase of the release cycle is the good timing to fix issues?
#28
@
3 years ago
- Keywords i18n-change added
I think it would be a good timing if these strings were new in 6.0, or somehow edited earlier in this release cycle, or somehow connected to the strings changed in [53118].
It's probably not much of an issue, just a matter of sticking to our own release schedule and something to keep in mind for future releases to avoid adding unexpected work for translators.
Adding the i18n-change
keyword as per Trac Workflow Keywords.
#29
@
3 years ago
Ok got it, thanks. To clarify: I didn't notice those strings were not new ones, introduced in 6.0 :)
#30
@
3 years ago
@SergeyBiryukov, @audrasjb
I'm concerned about leaving [53337] in 6.0 too.
How would it work if this was reverted:
- have the language packs been updated since RC1?
- fuzzy matching would pick up the strings from the 5.9 project, would polyglots need to re-approve the old strings?
#31
@
3 years ago
@peterwilsoncc it is already declared as a fuzzy string for WP 6.0.
See https://translate.wordpress.org/projects/wp/dev/admin/fr/default/?filters%5Bstatus%5D=either&filters%5Boriginal_id%5D=10187702&filters%5Btranslation_id%5D=75028029 for example.
I'm really sorry for the misundestanding and it is my mistake, but now, reverting the change would add even more work to polyglots teams.
related #37280