Skip to content

adding database server examples#3639

Merged
cmb69 merged 8 commits intophp:masterfrom
robertsilen:master
Aug 6, 2024
Merged

adding database server examples#3639
cmb69 merged 8 commits intophp:masterfrom
robertsilen:master

Conversation

@robertsilen
Copy link
Copy Markdown
Contributor

Adding database server examples that use the MySQL protocol (MariaDB, MySQL, Percona, TiDB) as per discussion #3628

Copy link
Copy Markdown
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! This looks generally good to me, also some issues still need to be resolved.

@kamil-tekiela, what do you think?

Comment thread reference/mysqlinfo/concepts.xml Outdated
Comment thread reference/mysqlinfo/concepts.xml Outdated
Comment thread reference/mysqlinfo/concepts.xml Outdated
Comment thread reference/mysqlinfo/concepts.xml Outdated
<para>
The MySQL extensions support the MySQL protocol.
Examples of compatible database servers are
<link xlink:href="https://mariadb.org/">MariaDB Server</link>,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The URLs should better be entities for easier updating across translations; see https://github.com/php/doc-base/blob/master/entities/global.ent (that is another repo, because it is used by all translations of the PHP manual).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added URLs now here, will change to variables when this is merged: #3639

Comment thread reference/mysqlinfo/set.xml
@cmb69
Copy link
Copy Markdown
Member

cmb69 commented Aug 5, 2024

Hmm, CI is reporting "Namespace prefix xlink for href on link is not defined"; not sure why, since apparently the respective XML namespace is declared.

Copy link
Copy Markdown
Member

@kamil-tekiela kamil-tekiela left a comment

Choose a reason for hiding this comment

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

It looks fine to me but I wouldn't duplicate the information. Pick one spot it fits the best.

@cmb69
Copy link
Copy Markdown
Member

cmb69 commented Aug 5, 2024

The build still fails, likely because of

<set xml:id="set.mysqlinfo">

The XML namespace declaration should probably be added to this element.

Copy link
Copy Markdown
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

This looks good to me now, but I think we should merge php/doc-base#144 first, and then adapt and merge this PR (might be a bit easier to handle for translators).

@cmb69
Copy link
Copy Markdown
Member

cmb69 commented Aug 6, 2024

Since php/doc-base#144 has been merged, could you please update the PR to use these entities?

@cmb69 cmb69 merged commit 67ddbaf into php:master Aug 6, 2024
@cmb69
Copy link
Copy Markdown
Member

cmb69 commented Aug 6, 2024

Thank you very much!

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.

3 participants