-
Notifications
You must be signed in to change notification settings - Fork 7
update readme #66
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
base: main
Are you sure you want to change the base?
update readme #66
Conversation
WalkthroughThe README.md file was extensively rewritten and expanded to provide detailed guidance for the Mailtrap Ruby client. New sections, enhanced usage examples, improved formatting, and clarifications were introduced, covering prerequisites, supported features, installation, usage scenarios, API examples, and encoding details. No code or public API changes were made. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
README.md (3)
24-33
: Fix nested-bullet indentation for consistent GitHub renderingThe second-level bullets are indented with two spaces; GitHub-flavoured Markdown only guarantees sub-list rendering when the indent is at least four spaces or one tab. On narrower screens this often collapses into a single list level, reducing readability.
-* **Email API/SMTP** - * Send an email (Transactional and Bulk streams) - * Send an email with a template - * Send a batch of emails (Transactional and Bulk streams) +* **Email API/SMTP** + * Send an email (Transactional and Bulk streams) + * Send an email with a template + * Send a batch of emails (Transactional and Bulk streams)(Apply the same 4-space indent to all nested items in Lines 25-33 and 35-43.)
79-88
: Avoid usingclient.send
in examples to prevent confusion withObject#send
Because
Object#send
is a core Ruby method, using the same name in documentation can mislead newcomers and static analyzers.
If the gem exposes an alias such asdeliver
/deliver_mail
, prefer it in docs; otherwise consider clarifying with a comment.# Prefer an alias if available client.deliver( from: sender, … )
93-101
: Show environment-based configuration instead of hard-coding secretsHard-coded API keys in README snippets risk copy-pasta into production. Demonstrate best practice by fetching from ENV:
# config/environments/production.rb config.action_mailer.delivery_method = :mailtrap config.action_mailer.mailtrap_settings = { api_key: ENV.fetch("MAILTRAP_API_KEY") }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md
(6 hunks)
🔇 Additional comments (1)
README.md (1)
257-261
: Confirmed:ContactsAPI#upsert
existsThe
upsert
method is defined in lib/mailtrap/contacts_api.rb (lines 50–53), so the README example is accurate and will not raise aNoMethodError
.
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.
Adjust example before merging, please
README.md
Outdated
TOKEN = "<YOUR-TOKEN-HERE>" | ||
SENDER_EMAIL = "<[email protected]>" | ||
RECIPIENT_EMAIL = "<[email protected]>" | ||
|
||
client = Mailtrap::Client.new(api_key: TOKEN) | ||
sender = { name: "Mailtrap Test", email: SENDER_EMAIL } | ||
|
||
client.send( | ||
from: sender, | ||
to: [{ email: RECIPIENT_EMAIL }], | ||
subject: "Hello from Mailtrap!", | ||
text: "Welcome to Mailtrap Sending!" | ||
) |
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.
There is a bit of inconsistency with further examples:
- we use
MAILTRAP_API_KEY
. Env var should have special meaning, but anyways better to use same terminology - SENDER_EMAIL and RECIPIENT_EMAIL do not have to be constant, in other examples they are not stored into variables at all
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.
hi. makes sense, I did some changes. is it better now?
- Update title alignment with main branch - Add API v2 documentation note - Enhance Email Templates API examples with full CRUD operations - Improve Contacts API examples with list management methods - Standardize API key usage: 'your-api-key' for simple examples, ENV.fetch for Rails - Fix nested bullet point indentation for proper GitHub rendering - Align examples with actual /examples folder implementations 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
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.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
README.md (1)
210-231
:email_template
is undefined – the example will raiseNameError
Lines 224-230 reference
email_template.id
, but the variable is never assigned. Readers copying the snippet will see a runtime failure.-templates.create( +email_template = templates.create( name: 'Welcome Email', ... ) # Get a specific template -templates.get(email_template.id) +templates.get(email_template.id)Capturing the return value (or using the ID returned by
create
) fixes the issue and keeps the workflow clear.
🧹 Nitpick comments (2)
README.md (2)
24-45
: Indent nested bullets with 2 spaces to satisfy common Markdown linters
markdownlint
(rule MD007) flags the sub-lists under “Supported functionality” because they’re indented by 4 spaces instead of 2. This breaks consistency with the rest of the README and some renderers.-* **Email API/SMTP** - * Send an email (Transactional and Bulk streams) - * Send an email with a template - * Send a batch of emails (Transactional and Bulk streams) +* **Email API/SMTP** + * Send an email (Transactional and Bulk streams) + * Send an email with a template + * Send a batch of emails (Transactional and Bulk streams) ...Applying a uniform 2-space indentation across all sub-bullets will silence the linter and improve readability.
65-81
: Show environment variable usage consistently across examplesMost later snippets promote
ENV.fetch('MAILTRAP_API_KEY')
, but the “Minimal” example hard-codes the key. Switching to the env-var form keeps terminology consistent (addressing an earlier review) and discourages committing secrets.-client = Mailtrap::Client.new(api_key: 'your-api-key') +client = Mailtrap::Client.new(api_key: ENV.fetch('MAILTRAP_API_KEY'))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md
(6 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
27-27: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
28-28: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
29-29: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
31-31: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
32-32: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
33-33: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
34-34: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
35-35: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
37-37: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
38-38: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
39-39: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
41-41: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
42-42: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
43-43: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
44-44: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
45-45: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🔇 Additional comments (1)
README.md (1)
233-275
: LGTM – examples are complete and correctly chainedThe Contacts API walkthrough covers list, field and contact CRUD in a logical order with valid arguments. Nice improvement over the previous docs.
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.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
README.md (1)
220-240
:email_template
variable is undefined in the Templates API example
templates.create
is not assigned to any variable, yet later calls referenceemail_template.id
, which will raiseNameError
if copied verbatim.-email_template = templates.create( +email_template = templates.create( name: 'Welcome Email', subject: 'Welcome to Mailtrap!', body_html: '<h1>Hello</h1>', body_text: 'Hello', category: 'welcome' ) # Get all templates templates.list # Get a specific template -templates.get(email_template.id) +templates.get(email_template.id)Capture the return value (as above) or switch subsequent calls to the correct variable name.
🧹 Nitpick comments (3)
README.md (3)
24-45
: Fix list indentation to comply with Markdown style guideThe nested “Supported functionality” bullets are indented 4 spaces instead of the 2-space indentation enforced by
markdownlint
(MD007). This is purely cosmetic but currently breaks style checks in some CI pipelines.-* **Email API/SMTP** - * Send an email (Transactional and Bulk streams) - * Send an email with a template - * Send a batch of emails (Transactional and Bulk streams) -* **Email Sandbox (Testing)** - * Send an email - * Send an email with a template - * Message management - * Inbox management - * Project management -* **Contact management** - * Contacts CRUD - * Lists CRUD - * Contact fields CRUD -* **General** - * Templates CRUD - * Suppressions management (find and delete) - * Account access management - * Permissions management - * List accounts you have access to +* **Email API/SMTP** + * Send an email (Transactional and Bulk streams) + * Send an email with a template + * Send a batch of emails (Transactional and Bulk streams) +* **Email Sandbox (Testing)** + * Send an email + * Send an email with a template + * Message management + * Inbox management + * Project management +* **Contact management** + * Contacts CRUD + * Lists CRUD + * Contact fields CRUD +* **General** + * Templates CRUD + * Suppressions management (find and delete) + * Account access management + * Permissions management + * List accounts you have access to
68-71
: Unify bullet style (“*” vs “-”)The README mostly uses
*
but this list uses-
, triggering MD004. Switching to asterisks keeps the formatting consistent.- - A [Mailtrap account](https://mailtrap.io/signup) - - A [verified domain](https://mailtrap.io/sending/domains) - - An API key from your Mailtrap dashboard +* A [Mailtrap account](https://mailtrap.io/signup) +* A [verified domain](https://mailtrap.io/sending/domains) +* An API key from your Mailtrap dashboard
85-92
: Discourage hard-coding secrets in examplesThe minimal example still inlines
"your-api-key"
. All earlier guidance encourages using theMAILTRAP_API_KEY
env var, so the snippet should follow the same pattern to avoid encouraging bad practice.-client = Mailtrap::Client.new(api_key: 'your-api-key') +client = Mailtrap::Client.new(api_key: ENV.fetch('MAILTRAP_API_KEY'))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md
(5 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
27-27: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
28-28: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
29-29: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
31-31: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
32-32: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
33-33: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
34-34: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
35-35: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
37-37: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
38-38: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
39-39: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
41-41: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
42-42: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
43-43: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
44-44: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
45-45: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
68-68: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
69-69: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
70-70: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
Updating a readme to have a bit the same structure as https://github.com/railsware/mailtrap-php
with Prerequisites and Supported functionality sections
Summary by CodeRabbit