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

Revise and add to Attribute's documentation #3441

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Kaiepi
Copy link
Collaborator

@Kaiepi Kaiepi commented May 28, 2020

The problem

Attribute is documented as being something you normally shouldn't use directly. I think this is useful to do when working with the MOP though, and I use Attribute like this in an example in the revision of one of the role HOWs' type pages (#3353).

Solution provided

  • Reorganize Attribute's type page somewhat and remove some unnnecessary "Defined as:" before each method signature.
  • Show how Attribute can be used when working with the MOP.
  • Document Attribute.new, along with Attribute.is_bound, and Attribute.is_built.
  • Document use of :bind with the is built trait, as this is referred to by the new documentation for Attribute.new.

@Kaiepi Kaiepi requested a review from lizmat May 28, 2020 17:13
Copy link
Contributor

@JJ JJ left a comment

Choose a reason for hiding this comment

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

I need to finish now, but can you please check the comments? Also #2745 is probably the issue you want to target

For instance, the names of all the attributes of a type can be introspected
using its L<name|#method_name> method:

=begin code :solo
Copy link
Contributor

Choose a reason for hiding this comment

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

Why :solo? This would compile correctly, right? And it does not need its own file.

doc/Type/Attribute.pod6 Show resolved Hide resolved

Because of C<Attribute>, a type containing attributes, such as this:

=for code :solo
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, not needed.

Copy link
Contributor

@JJ JJ left a comment

Choose a reason for hiding this comment

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

I understand this is still a work in progress, right?

doc/Type/Attribute.pod6 Show resolved Hide resolved
since Attribute is at the level of the metaclass, all is fair game.
=head2 method new

=for code :skip-test<too long a method signature for one line>
Copy link
Contributor

Choose a reason for hiding this comment

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

In principle, there should be no problem with so many lines, as long as you use =for code or =begin code


Creates a new attribute. The following named arguments are required:

- C<$name> contains the attribute's name, which should always be a
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe use =item here?

doc/Type/Attribute.pod6 Show resolved Hide resolved
@Kaiepi
Copy link
Collaborator Author

Kaiepi commented Nov 19, 2022

  1. Tests are solo'd in this because of symbol clashes, which looks to allow typos like :1has_accessor already, so those examples need redoing.
  2. I thought it was common style in the docs to put the code block under the header without the Defined as: before, but it looks to be mixed. Is there any preference?

@coke
Copy link
Collaborator

coke commented Nov 20, 2022

I don't think we need "Declared as:" going forward - having the signature as the first line seems obvious enough. Looks like Mu is the only place this was used.

@coke coke mentioned this pull request Nov 24, 2022
@coke coke self-assigned this Feb 2, 2023
@coke coke added this to the 2023-First Quarter milestone Feb 7, 2023
@coke coke removed their assignment Mar 4, 2023
@coke coke modified the milestones: 2023-Quarter 1, 2023-Quarter 2 Apr 1, 2023
@coke coke modified the milestones: 2023-Quarter 2, 2023-Quarter 3 Jul 12, 2023
@coke coke modified the milestones: 2023-Quarter 3, 2023-Quarter 4 Oct 8, 2023
@coke coke modified the milestones: 2023-Quarter 4, 2024-Quarter-2 Mar 31, 2024
@coke coke modified the milestones: 2024-Quarter-2, 2024-Quarter-3 Jun 19, 2024
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