Opened 6 months ago
Last modified 6 months ago
#58102 new defect (bug)
Check PHPcs Coding standard into the wp-includes folder
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | |
Component: | General | Keywords: | |
Focuses: | coding-standards | Cc: |
Description
Hello Team,
I have reviewed the code and found a few errors in some of the below files. Please below files:
Files:
wp-includes/admin-bar.php
wp-includes/option.php
Thanks,
Attachments (13)
Change History (19)
#2
@
6 months ago
There is a small issue in your change on revison.php: __()
should not be replaced with esc_html_e()
but rather with esc_html__()
.
#3
@
6 months ago
Also adding esc_attr()
to $type_attr
in
<style<?php echo $type_attr; ?> media="print">#wpadminbar { display:none; }</style>
looks wrong to me, as $type_attr
may contain an attribute + its value (example: type="text/css"
.
If we want to escape this variable, I think esc_html
would be more appropriate here.
#4
follow-up:
↓ 5
@
6 months ago
Shouldn't all the changes be in one diff file instead of multiple, like 58102.diff, 58102.2.diff etc. Or is that okay to have multiple diff files?
Because in the 58102.diff, there is a change in the option.php file. In 58102.2.diff, there is a change in admin-bar.php and so on.
#5
in reply to:
↑ 4
@
6 months ago
Replying to krupalpanchal:
Shouldn't all the changes be in one diff file instead of multiple, like 58102.diff, 58102.2.diff etc. Or is that okay to have multiple diff files?
Because in the 58102.diff, there is a change in the option.php file. In 58102.2.diff, there is a change in admin-bar.php and so on.
I think it's best practice to set them all in one file. Otherwise it looks like we have 4 different versions of one patch...
#6
@
6 months ago
It really depends on the scope of the patch. Sometimes, it's easier to separate things into multiple small scoped patches.
But yes, when adding multiple patches to one ticket, it's probably better to name them after to scope of the change.
Example: 58102-revisions.diff
would probably be a better name for 58102.4.diff
.
@
5 months ago
I have removed unused variables from above mentioned files and checked phpcs coding standard.
I have checked above mentioned issue and I have resolved it and added patch.