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
Next Next commit
[Issue : #236] : Documentation for Email class was made more concise
  • Loading branch information
snehitgajjar committed Jun 15, 2016
commit e9ac47bccc59e0226582fa5c379dd00c48b5e3c2
15 changes: 10 additions & 5 deletions lib/nexpose/common.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Copy link
Contributor

Choose a reason for hiding this comment

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

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 :send_as
# Send to all the authorized users of sites, groups, and assets.
# Ex. : Fixnum [1 | 0]
attr_accessor :to_all_authorized
# Send to users on the report access list.
# Send to users on the report access list. Ex. : Fixnum [1 | 0]
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.
# One of: [String] 'file' | 'zip'
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.

Same here. Also note that url is an option for this one.

attr_accessor :send_to_owner_as

# Sender that e-mail will be attributed to.
# String ['abc.123@gmail.com']
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be:

[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 ['abcd.server.com']
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be:

[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).
# Array[String] of report recipients (i.e., not already on the report access list).
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.

Change this to:

[Array<String>]

for the right highlighting in generated documentation. Probably want to specify the recipients are expected as email addresses. (I'll need to double check the syntax on this actually)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Syntax of email address is String each one per new line. I have made sure still take a look at it.

attr_accessor :recipients

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