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

Added basic SMTP documentation and examples #751

Merged
merged 1 commit into from
Aug 3, 2014
Merged

Added basic SMTP documentation and examples #751

merged 1 commit into from
Aug 3, 2014

Conversation

etcimon
Copy link
Contributor

@etcimon etcimon commented Jul 29, 2014

The documentation was a little underwhelming in the SMTP module, and I thought the sendmail example in the examples/ directory was quite unhelpful as a quick reference. So, this change serves its purpose of explaining how to send a basic message through sendMail, so that anyone can get his solution up within a couple minutes.

@s-ludwig
Copy link
Member

Thanks, you are right, documentation is a bit thin there. Two corrections/improvements for the doc comment:

  • The first paragraph is also the short description of the symbol, so the most simple correction would be to put the first sentence on its own paragraph.

  • The example should be in a documented unit test, so that it is automatically checked to compile:

    ///
    unittest {
        void test()
        {
            auto settings = new SMTPClientSettings(...);
            // ...
        }
    }
    

Also, do you think that sending the mail from within a HTTP request handler adds any value to the documentation? It seems to be a bit out of place to me when looking at it from a "neutral" point of view.

@etcimon
Copy link
Contributor Author

etcimon commented Aug 1, 2014

Also, do you think that sending the mail from within a HTTP request handler adds any value to the documentation? It seems to be a bit out of place to me when looking at it from a "neutral" point of view.

I probably over-did it, I would surely be better to remove the variables altogether.

The first paragraph is also the short description of the symbol, so the most simple correction would be to put the first sentence on its own paragraph.

The example should be in a documented unit test, so that it is automatically checked to compile:

So the /// registers this unit test as an example?

@s-ludwig
Copy link
Member

s-ludwig commented Aug 1, 2014

Exactly, you could also write some text that would appear before the example. This is a relatively recent feature of DMD/Ddoc (not sure if 2.064 or 2.065) that is now also supported for DDOX.

Added SMTP unittest as an example

Fixed imports and added example explanations

Added another import
@etcimon
Copy link
Contributor Author

etcimon commented Aug 3, 2014

This should be passing, the logs seem to complain about the Redis test

@s-ludwig
Copy link
Member

s-ludwig commented Aug 3, 2014

The failure looks strange, but your changes indeed look good. I'll merge and correct the documentation comment.

s-ludwig added a commit that referenced this pull request Aug 3, 2014
Added basic SMTP documentation and examples
@s-ludwig s-ludwig merged commit 4006dce into vibe-d:master Aug 3, 2014
s-ludwig added a commit that referenced this pull request Aug 3, 2014
@etcimon
Copy link
Contributor Author

etcimon commented Aug 3, 2014

Thanks !

The first paragraph is also the short description of the symbol, so the most simple correction would be to put the first sentence on its own paragraph.

I must have not understood this, which symbol does it describe?

@s-ludwig
Copy link
Member

s-ludwig commented Aug 3, 2014

The first paragraph of a Ddoc comment (where a paragraph is a set of one or more lines delimited by one or more empty lines) is defined to be the "short summary" for the documented symbol as per the Ddoc specification. It is used by DDOX to populate the overview tables. This first paragraph should be relatively short, around maybe 60 to 120 characters.

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.

2 participants