Skip to content

Add chapter on Attributes in language section.#254

Closed
beberlei wants to merge 3 commits intophp:masterfrom
beberlei:Attributes
Closed

Add chapter on Attributes in language section.#254
beberlei wants to merge 3 commits intophp:masterfrom
beberlei:Attributes

Conversation

@beberlei
Copy link
Copy Markdown
Contributor

@beberlei beberlei commented Nov 30, 2020

I am not particularly good at documneting things, but this is an acceptable start I hope, for the Attributes feature. it covers the basics:

  • Syntax
  • Reflection API
  • Declaring Attribute Classes

A bunch of things are missing for now:

  • Better Examples, not Thing and such
  • ReflectionAttribute class in reflection book
  • getArguments() methods in every relevant reflection component
  • Explanation of internal attributes declared in Core and Extensions

after merging this, doc-base needs a small patch to render this:

diff --git a/manual.xml.in b/manual.xml.in
index 6e59239d..6c8b1ba8 100644
--- a/manual.xml.in
+++ b/manual.xml.in
@@ -80,6 +80,7 @@
   &language.errors;
   &language.exceptions;
   &language.generators;
+  &language.attributes;
   &language.references;
   &language.predefined.variables;
   &language.predefined.exceptions;

Copy link
Copy Markdown
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

Can't comment on the general text as I don't really understand the feature, but the usage of you should be avoided in the docs. So if you could change that that would be great. :)

Comment on lines +11 to +12
on declarations in code: Classes, methods, functions, parameters,
properties and constants can be the target of an attribute. The metadata
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.

Should we add links to the relevant pages?

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.

Might be more noise than actually being useful.

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! Besides the few nits, I think this is good to be merged. The rest of the documentation updates can be done later.

Co-authored-by: Christoph M. Becker <cmbecker69@gmx.de>
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