[Issue : #236] : Documentation for Email class was made more concise #237
Conversation
| @@ -25,20 +25,25 @@ module Scope | |||
| # non-members. | |||
| class Email | |||
| # Send as file attachment or zipped file to individuals who are not members | |||
| # of the report access list. One of: file|zip | |||
| # of the report access list. One of: [String] 'file' | 'zip' | |||
There was a problem hiding this comment.
Might be clearer as:
[String] the attachment format, `file` or `zip`.
Possibly on a new line rather than being the end of the 2nd line.
| attr_accessor :to_all_authorized | ||
| # Send to users on the report access list. | ||
| # Send to users on the report access list. |
| attr_accessor :recipients | ||
|
|
||
| attr_accessor :sender | ||
| # SMTP relay server. | ||
| # SMTP relay server. This is necessary to avoid security issue for |
There was a problem hiding this comment.
What is the security comment about? The way I understand it is that SMTP relay server is not necessarily for security reasons, but for general mail processing reasons (spam filtering, control of data transmitted over network, etc).
There was a problem hiding this comment.
I generically wrote security reasons to specify spam and data transmission. If you suggest I can be more specific.
There was a problem hiding this comment.
I think in the context of this documentation it is unnecessary to include at all. Whether or not an SMTP server is required, and why it is or is not required, would be an outside concern (i.e. general Nexpose product documentation).
The documentation here should just be about using the Ruby code. If anything, it could note that the SMTP relay server would be used for sending the emails to the recipients (if that's not obvious/clear to anyone using this).
There was a problem hiding this comment.
I believe that. Will remove it.
| attr_accessor :smtp_relay_server | ||
| # Array of report recipients (i.e., not already on the report access list). | ||
| # Recipients will be in form of email address. | ||
| # [Array<String>] of report recipients (i.e., not already on the report access list). |
There was a problem hiding this comment.
I suggest changing this to a single line:
# [Array<String>] E-mail addresses of additional report recipients (i.e., not already on the report access list).
|
Aside from my last 2 comments, I think this looks good. Thanks for doing this! cc @sgreen-r7 should take a look as well. |
|
Looks good to me as well! Thank you a million for this @snehitgajjar |
@gschneider-r7 Please review. I have made changes in Email class to make class variable description more concise. Resolves #236