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

Add chapter on Attributes in language section. #254

Closed
wants to merge 3 commits into from

Conversation

beberlei
Copy link
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
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
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
Contributor

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.

language/attributes.xml Outdated Show resolved Hide resolved
language/attributes.xml Outdated Show resolved Hide resolved
language/attributes.xml Outdated Show resolved Hide resolved
language/attributes.xml Outdated Show resolved Hide resolved
language/attributes.xml Outdated Show resolved Hide resolved
language/attributes.xml Outdated Show resolved Hide resolved
language/attributes.xml Outdated Show resolved Hide resolved
language/attributes.xml Outdated Show resolved Hide resolved
Copy link
Contributor

@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.

language/attributes.xml Outdated Show resolved Hide resolved
language/attributes.xml Outdated Show resolved Hide resolved
language/attributes.xml Outdated Show resolved Hide resolved
language/attributes.xml Outdated Show resolved Hide resolved
language/attributes.xml Outdated Show resolved Hide resolved
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.

None yet

3 participants