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

[Issue : #236] : Documentation for Email class was made more concise #237

Merged
merged 7 commits into from
Jun 15, 2016
18 changes: 13 additions & 5 deletions lib/nexpose/common.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,28 @@ 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.
# [String] Attachment format, 'file' | 'zip'.
attr_accessor :send_as
# Send to all the authorized users of sites, groups, and assets.
# [Fixnum] 1 | 0
attr_accessor :to_all_authorized
# Send to users on the report access list.
# Send to users on the report access list.

Choose a reason for hiding this comment

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

Trailing whitespace detected.

# [String] Attachment format 'file' | 'zip'
attr_accessor :send_to_acl_as
# Format to send to users on the report access list. One of: file|zip|url
# Format to send to users on the report access list.
# [String] Attachment format 'file' | 'zip' | 'url'
attr_accessor :send_to_owner_as

# Sender that e-mail will be attributed to.
# [String] an email address
attr_accessor :sender
# SMTP relay server.
# SMTP relay server. This is necessary to avoid security issue for
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I generically wrote security reasons to specify spam and data transmission. If you suggest I can be more specific.

Copy link
Contributor

@gschneider-r7 gschneider-r7 Jun 15, 2016

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe that. Will remove it.

# internal network.
# [String] the IP address, host name or FQDN of the SMTP server.
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).
Copy link
Contributor

Choose a reason for hiding this comment

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

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

attr_accessor :recipients

def initialize(to_all_authorized, send_to_owner_as, send_to_acl_as, send_as)
Expand Down