Skip to content

Improve documentation for Identifier field type - #720

Open
AlwaysVictorious wants to merge 2 commits into
PHP-CMSIG:0.12from
AlwaysVictorious:patch-10
Open

Improve documentation for Identifier field type#720
AlwaysVictorious wants to merge 2 commits into
PHP-CMSIG:0.12from
AlwaysVictorious:patch-10

Conversation

@AlwaysVictorious

Copy link
Copy Markdown
Contributor

Clarified the description of the Identifier field type and its requirements.

Clarified the description of the Identifier field type and its requirements.

@alexander-schranz alexander-schranz left a comment

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 changed text is not correct as Identifiers has its own defaults. So the last sentence should not be changed to reference the TextField type as its not true.

@alexander-schranz alexander-schranz added the documentation Improvements or additions to documentation label Aug 6, 2026
@alexander-schranz

Copy link
Copy Markdown
Member

PS: Thx for the effort, but it would help if you would create a pull request containing all your suggested docs changes instead per file, as we can then do a combined review instead have todo multiple reviews.

@AlwaysVictorious

AlwaysVictorious commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

I had to reread the documentation and now I see that there is a "options" section indeed. The problem I'm having (and that I will edit sometime soon but probably not today) is with "The Identifier field type is a special Text field type." This suggests extension hence there should be at least as many options to the Identifier field as to the Text field. I quickly looked for a table, didn't see any, read "The defaults can not be changed and so are same for every index." and thought that hence it would use the Text field defaults.

PR 721 addresses as part of it a similar issue. I will change this, change the offending sentence and will an options table here so that it matches the rest of the options sections. I will do this tomorrow I think

@AlwaysVictorious

AlwaysVictorious commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

PS: Thx for the effort, but it would help if you would create a pull request containing all your suggested docs changes instead per file, as we can then do a combined review instead have todo multiple reviews.

To be honest, I did not plan on doing any of this. I just try to read the documentation and then when something can be improved, I do that expecting it to be one minor thing. And then there is another minor thing. And another. From now on, if I encounter more changes, I will bundle them. There is however one PR I already made (#721)

Clarified the description of the IdentifierField type and its options.

@AlwaysVictorious AlwaysVictorious left a comment

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.

Feel free to ask for other changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants