-
Notifications
You must be signed in to change notification settings - Fork 19
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
Enhance MinFraud Documentation: Add Detailed Information on Available Options #152
Conversation
README.md
Outdated
@@ -84,7 +84,7 @@ and [Report Transaction](https://dev.maxmind.com/minfraud/report-transaction/) A | |||
### minFraud API ### | |||
|
|||
To use the minFraud API, create a new `\MaxMind\MinFraud` object. The constructor | |||
takes your MaxMind account ID, license key, and an optional options array as | |||
takes your MaxMind account ID, license key, and an optional [options](#Options) array as |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
README.md
Outdated
|
||
| Option | Description | | ||
|------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | ||
| **host** | The host to use when connecting to the web service. By default, the client connects to the production host. However, during testing and development, you can set this option to '[sandbox.maxmind.com](sandbox.maxmind.com)' to use the sandbox host. The sandbox host allows you to experiment with the API without affecting your production data. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this kind of stuff would belong in the API docs. See here for how we mention the alternate host for the GeoIP2-php API.
44f451c
to
247bfc0
Compare
247bfc0
to
f1d4476
Compare
README.md
Outdated
takes your MaxMind account ID, license key, and an optional options array as | ||
arguments. This object is immutable. You then build up the request using the | ||
`->with*` methods as shown below. Each method call returns a new object. The | ||
To use the MinFraud API, you'll need to create a new `MaxMind\MinFraud` object. The constructor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why'd we lose the \
? Can we keep this as minFraud API
? The original phrasing seems fine.
README.md
Outdated
]; | ||
|
||
// Create a MinFraud object with your account ID, license key, and options | ||
$mf = new MaxMind\MinFraud(1, 'ABCD567890', $options); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be just new MinFraud
I think - see example below.
README.md
Outdated
arguments. This object is immutable. You then build up the request using the | ||
`->with*` methods as shown below. Each method call returns a new object. The | ||
To use the MinFraud API, you'll need to create a new `MaxMind\MinFraud` object. The constructor | ||
requires your MaxMind account ID, license key, and an optional [`options`](https://maxmind.github.io/minfraud-api-php/doc/v1.23.0/classes/MaxMind-MinFraud.html#method___construct) array as arguments. The `options` array can be used to set additional parameters such as `host` and `timeout`. Remember that this object is immutable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, having the version number in the URL is not ideal. I think that was a poor suggestion on my part. Maybe we can refer to the API documentation generally instead, e.g. "See the API documentation for the possible options."
README.md
Outdated
<?php | ||
// Set any options you need, such as the host for the Sandbox environment | ||
$options = [ | ||
'host' => 'sandbox.maxmind.com' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we update the API docs to discuss the sandbox host too, like we do for the geolite host for the GeoIP2 APIs? That is, we could expand the comment about the host option.
|
||
// Now you can use the $mf object to interact with the MinFraud API | ||
?> | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make this more succinct? This is a big block in the summary section of the docs. I think it would be good to keep this section as clear and simple as possible.
e.g.
For instance, to call the Sandbox environment instead of the minFraud 2 web service:
$client = new MinFraud(1, 'ABCD567890', [ 'host' => 'sandbox.maxmind.com' ]);
635afce
to
a5300be
Compare
src/MinFraud.php
Outdated
@@ -62,6 +62,10 @@ class MinFraud extends MinFraud\ServiceClient | |||
* @param array $options An array of options. Possible keys: | |||
* | |||
* * `host` - The host to use when connecting to the web service. | |||
* By default, the client connects to the production host. However, | |||
* during testing and development, you can set this option to | |||
* 'sandbox.maxmind.com' to use the sandbox host. The sandbox host |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about:
"to use the Sandbox environment's host. The Sandbox allows you"
README.md
Outdated
takes your MaxMind account ID, license key, and an optional `options` array as | ||
arguments. This object is immutable. See the API documentation for the possible options. | ||
|
||
For instance, to call the Sandbox environment instead of the minFraud 2 web service: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about:
"For instance, to use the Sandbox web service instead of the production web service, you can provide the host
option:"
I realize I suggested the original but it reads funny to me today 😿
I'm not sold on "production" as a widely recognizable term, but I see we use it in the support site docs too, so I guess it's fine.
c49b5c2
to
375650e
Compare
Summary
This pull request enhances the documentation, providing valuable information on the
options
parameter usage.Changes Made
I have added an extensive section about the available options parameter in the README.md file. This section now explains each option in detail, allowing users to customize the MinFraud client's behavior effectively.