Skip to content

Add Ddoc headers for front-end modules#10903

Merged
dlang-bot merged 5 commits intodlang:masterfrom
dkorpel:documentModules
Mar 28, 2020
Merged

Add Ddoc headers for front-end modules#10903
dlang-bot merged 5 commits intodlang:masterfrom
dkorpel:documentModules

Conversation

@dkorpel
Copy link
Contributor

@dkorpel dkorpel commented Mar 12, 2020

As suggested by Walter: #9844 (comment)

  • Adds missing module descriptions
  • Consistently puts the description above the boilerplate
  • Adds links to the specification where applicable

The descriptions are sometimes vague since I'm not always sure myself what each module does exactly, but it's a start.

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @dkorpel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#10903"

@Geod24
Copy link
Member

Geod24 commented Mar 13, 2020

I would just remove the boilerplate. Especially with the way you put it, it's part of the "Specification" section, not the description.

Copy link
Member

@Geod24 Geod24 left a comment

Choose a reason for hiding this comment

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

Included few comments (didn't finish my review).

In general:

  • Get rid of the boilerplate
  • Make sure you have an empty line between the title and the description.
  • Make sure the descriptions comes directly after the title, not after a section.

@@ -1,4 +1,8 @@
/**
* Implement array opterations, such as `a[] = b[] + c[]`.
Copy link
Member

Choose a reason for hiding this comment

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

Typo

@@ -1,4 +1,6 @@
/**
* Defines an abstract ASTNode class.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Defines an abstract ASTNode class.
* Defines the base class for all nodes which are part of the AST.

module dmd.astbase;

/**
*/
Copy link
Member

Choose a reason for hiding this comment

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

?

src/dmd/attrib.d Outdated
@@ -1,4 +1,17 @@
/**
* Defines declarations of various 'attributes'.
* The term attribute is used very loosly, since it includes:
Copy link
Member

Choose a reason for hiding this comment

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

Put an empty line (well, with a *) between line 2 and 3 to make this part of the description.
Also I wouldn't say it is not used loosely, just that your expectation of what an attribute is might differ.
It has more to do with things applying to a larger scope than a single declaration, I'd say.

@@ -1,4 +1,7 @@
/**
* Evaluate certain external functions for CTFE.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Evaluate certain external functions for CTFE.
* Implement CTFE for intrinsic (builtin) functions

Also the empty line to separate title from description.
You might want to link / read https://en.wikipedia.org/wiki/Intrinsic_function

src/dmd/cli.d Outdated
* Compiler implementation of the
* $(LINK2 http://www.dlang.org, D programming language).
*
* This modules defines the help texts for the CLI options offered by DMD.
Copy link
Member

Choose a reason for hiding this comment

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

Newline after this to make the title short.

@@ -1,4 +1,9 @@
/**
* Does name mangling for D symbols, which generates a unique name for a linker symbol based
Copy link
Member

Choose a reason for hiding this comment

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

Too long, and inconsistent with the cppmangle side which does not explain what mangling is.
But really, I would expect anyone working on a compiler to know that. We can still add a See_Also to wikipedia if you feel like it, but we shouldn't paraphrase it.

src/dmd/attrib.d Outdated
* The term attribute is used very loosly, since it includes:
* Defines declarations of various attributes.
*
* The term 'attribute' refers to things that apply to a larger scope than a single declaration.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* The term 'attribute' refers to things that apply to a larger scope than a single declaration.
* The term 'attribute' refers to things that can apply to a larger scope than a single declaration.

E.g. depending on how you declare it, you might have deprecated as an attribute which is a parent of Dsymbols, or directly as part of the Dsymbol's storage classes.

/**
* Does name mangling for D symbols, which generates a unique name for a linker symbol based
* on the namespace, identifier and type.
* Does name mangling for D symbols.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Does name mangling for D symbols.
* Does name mangling for `extern(D)` symbols.

@dkorpel
Copy link
Contributor Author

dkorpel commented Mar 13, 2020

I would just remove the boilerplate. Especially with the way you put it, it's part of the "Specification" section, not the description.

Do you mean this text:

Compiler implementation of the $(LINK2 http://www.dlang.org, D programming language)

Or the other links as well? Except for 'Authors', all default sections could be auto-generated, but I do think Walter likes to have them in the source file.

@Geod24
Copy link
Member

Geod24 commented Mar 13, 2020

Do you mean this text:

Yep just this. The rest is fine.

@dlang-bot dlang-bot merged commit d2e8a54 into dlang:master Mar 28, 2020
@dkorpel dkorpel deleted the documentModules branch March 28, 2020 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants