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

[Documentation]: WordPress.DB.PreparedSQL #2454

Merged
merged 4 commits into from
Jul 23, 2024

Conversation

jaymcp
Copy link
Contributor

@jaymcp jaymcp commented Jun 13, 2024

Description

This PR adds documentation for the WordPress.DB.PreparedSQL sniff.

Related issues/external references

Part of #1722

@GaryJones
Copy link
Member

Have a look at some of the existing examples, such as in https://github.com/WordPress/WordPress-Coding-Standards/tree/develop/WordPress/Docs/Arrays to see how the <em> </em> tags are used to emphasise the key bit of the code examples.

@jaymcp
Copy link
Contributor Author

jaymcp commented Jun 20, 2024

Thanks @GaryJones, I've addressed the missing emphasis in fdd28c7.

Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

@jaymcp Hi Jay,

Thanks for working on this! Looking good!

I've gone through it with a critical eye and while the code samples were correct as they were, I have a feeling changing them a little would make the problem the sniff flags more obvious for people to grasp.

As, with the changes I made, some lines would become too long for code samples in docs, I've also broken up the SQL queries over multiple lines.

Let me know what you think about these suggestions.

WordPress/Docs/DB/PreparedSQLStandard.xml Outdated Show resolved Hide resolved
WordPress/Docs/DB/PreparedSQLStandard.xml Outdated Show resolved Hide resolved
WordPress/Docs/DB/PreparedSQLStandard.xml Outdated Show resolved Hide resolved
WordPress/Docs/DB/PreparedSQLStandard.xml Outdated Show resolved Hide resolved
WordPress/Docs/DB/PreparedSQLStandard.xml Outdated Show resolved Hide resolved
WordPress/Docs/DB/PreparedSQLStandard.xml Outdated Show resolved Hide resolved
WordPress/Docs/DB/PreparedSQLStandard.xml Outdated Show resolved Hide resolved
jaymcp and others added 2 commits July 1, 2024 18:28
Rewrites code examples to be clearer, and adds highlighting. Props: @jrfnl

Co-authored-by: Juliette <[email protected]>
@jaymcp
Copy link
Contributor Author

jaymcp commented Jul 1, 2024

Thanks @jrfnl, I appreciate the thorough feedback. I have committed all of your suggestions 🙂

Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

Thanks for the update @jaymcp and sorry for my slow response.

All good now. As far as I'm concerned, this is ready for merge.

For whomever merges this: please squash-merge.

@dingo-d dingo-d merged commit 32fe3c4 into WordPress:develop Jul 23, 2024
37 checks passed
@jaymcp jaymcp deleted the docs/WordPress.DB.PreparedSQL branch July 31, 2024 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants