Skip to content

Conversation

capcom6
Copy link
Member

@capcom6 capcom6 commented Sep 17, 2025

Summary by CodeRabbit

  • Documentation

    • Revamped README with "About The Project", new "Key value propositions", narrative overview, and reorganized Features.
    • Overhauled table of contents, clarified requirements with HTTP client options and optional dependency note, added security best practices and secure configuration examples.
    • Updated badges for issues, stars, forks, and added a PRs badge.
  • Chores

    • Updated public usage examples and exports in docs: explicit client/http imports, clarified sync vs async HTTP client naming, and top-level Encryptor export.

Copy link

coderabbitai bot commented Sep 17, 2025

Walkthrough

README.md rewritten: badges and top-level structure updated, narrative "About The Project" and "Key value propositions" added, Features and Table of Contents reorganized, Requirements clarified (HTTP client choices, optional pycryptodome), usage/examples and public API references adjusted (imports and async client name), security guidance added; no source code changes.

Changes

Cohort / File(s) Summary of Changes
Documentation
README.md
Full README overhaul: updated badges to android-sms-gateway/client-py and added CodeRabbit PR badge; replaced original Features with "About The Project" and "Key value propositions" (reintroducing an expanded Features section later); reorganized Table of Contents and anchors; clarified Requirements including explicit "HTTP Client (choose one)" and Optional Dependencies (pycryptodome); updated usage examples to from android_sms_gateway import client, http and to reference http.AsyncHttpClient (async) and http.HttpClient (sync); showed Encryptor as a top-level export in examples; added Security best practices and secure configuration snippet.

Sequence Diagram(s)

(omitted — changes are documentation-only and do not modify control flow)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR's primary change is a comprehensive rework of README.md (restructured content, examples, and API notes) as shown in the raw summary, and the title "[docs] unify README" is concise, focused, and accurately signals that this is a documentation consolidation. It is clear and appropriate for teammates scanning PR history.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch docs/unify-readme

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 381c9cd and 50c2b29.

📒 Files selected for processing (1)
  • README.md (5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • README.md

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
README.md (3)

29-61: Broken ToC anchors for emoji headers (leading “-” in links).
GitHub drops emojis in slugs; anchors shouldn’t start with “-”. Fix links to their actual slugs.

- - [📖 About The Project](#-about-the-project)
- - [📚 Table of Contents](#-table-of-contents)
-   - [✨ Features](#-features)
-   - [⚙️ Requirements](#️-requirements)
-   - [📦 Installation](#-installation)
+ - [📖 About The Project](#about-the-project)
+ - [📚 Table of Contents](#table-of-contents)
+   - [✨ Features](#features)
+   - [⚙️ Requirements](#requirements)
+   - [📦 Installation](#installation)
    - [Basic Installation](#basic-installation)
    - [Installation with Specific HTTP Client](#installation-with-specific-http-client)
    - [Installation with Encryption](#installation-with-encryption)
-   - [🚀 Quickstart](#-quickstart)
+   - [🚀 Quickstart](#quickstart)
    - [Initial Setup](#initial-setup)
    - [Encryption Example](#encryption-example)
-   - [🤖 Client Guide](#-client-guide)
+   - [🤖 Client Guide](#client-guide)
    - [Client Configuration](#client-configuration)
    - [Available Methods](#available-methods)
    - [Data Structures](#data-structures)
      - [Message](#message)
      - [MessageState](#messagestate)
      - [Webhook](#webhook)
-   - [🌐 HTTP Clients](#-http-clients)
+   - [🌐 HTTP Clients](#http-clients)
    - [Using Specific Clients](#using-specific-clients)
    - [Custom HTTP Client](#custom-http-client)
-   - [🔒 Security](#-security)
+   - [🔒 Security](#security)
    - [Best Practices](#best-practices)
    - [Secure Configuration Example](#secure-configuration-example)
-   - [📚 API Reference](#-api-reference)
-   - [👥 Contributing](#-contributing)
+   - [📚 API Reference](#api-reference)
+   - [👥 Contributing](#contributing)
    - [How to Contribute](#how-to-contribute)
    - [Development Environment](#development-environment)
    - [Pull Request Checklist](#pull-request-checklist)
-   - [📄 License](#-license)
-   - [🤝 Support](#-support)
+   - [📄 License](#license)
+   - [🤝 Support](#support)

Also change the very first ToC entry:

- [📱 SMS Gateway for Android™ Python API Client](#-sms-gateway-for-android-python-api-client)
+ [📱 SMS Gateway for Android™ Python API Client](#sms-gateway-for-android-python-api-client)

268-279: Import bug in example: client is used but not imported.

-from android_sms_gateway import http
+from android_sms_gateway import client, http

339-341: Pipenv command is invalid (--categories).
Use editable install with extras inside the virtualenv.

-# Create virtual environment
-pipenv install --dev --categories encryption,requests
-pipenv shell
+# Create and activate dev environment
+pipenv install --dev
+pipenv shell
+
+# Install package locally with extras
+python -m pip install -e '.[requests,encryption]'
🧹 Nitpick comments (3)
README.md (3)

16-27: Tighten claims around “Robust Security.”
“Secure credential management” is mostly a usage guideline (env vars), not a library feature. Consider softening to avoid over‑promising.

-🛡️ **Robust Security** - Built-in support for secure credential management and end-to-end encryption
+🛡️ **Robust Security** - Guidance for secure credential handling and optional end‑to‑end encryption

283-283: Likely typo: ahttp.HttpClient.
If the async protocol is exposed under http, reference that instead.

-Implement your own HTTP client following the `http.HttpClient` or `ahttp.HttpClient` protocols.
+Implement your own HTTP client following the `http.HttpClient` (sync) or `http.AsyncHttpClient` (async) protocols.

If types differ, please align wording with the actual protocol/class names exported by the package.


82-84: Map pycryptodome to the [encryption] extra in README

README.md (lines 82–84) — make explicit that pycryptodome is pulled in by the [encryption] optional extra (pyproject.toml already declares encryption = ["pycryptodome"]).

-**Optional Dependencies**:
-- 🔒 [pycryptodome](https://pypi.org/project/pycryptodome/) - For end-to-end encryption support
+**Optional Dependencies**:
+- 🔒 [pycryptodome](https://pypi.org/project/pycryptodome/) — pulled in by the `[encryption]` extra
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f32953f and e98ba39.

📒 Files selected for processing (1)
  • README.md (2 hunks)
🔇 Additional comments (2)
README.md (2)

7-9: Badges now point to the correct repo — looks good.


65-73: Feature list reads well and matches later sections.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
README.md (1)

268-279: Top-level exports and class names: fix package export and missing async client

  • android_sms_gateway/init.py does not export http (it imports HttpClient and all lists "APIClient","AsyncAPIClient"); add from . import http and include 'http' in all, or update the README to import from android_sms_gateway.http.
  • android_sms_gateway/http.py defines RequestsHttpClient and HttpxHttpClient but no AiohttpHttpClient was found; implement AiohttpHttpClient or remove/update the async example in README.
🧹 Nitpick comments (2)
README.md (2)

14-27: Naming consistency: use a single brand form.
Line 16 says “SMSGate” while the title/links use “SMS Gateway for Android” / “sms-gate.app”. Pick one and use it throughout to avoid confusion.

-The Python client for SMSGate provides a clean, type-safe interface...
+The Python client for SMS Gateway for Android provides a clean, fully‑typed interface...

62-73: Feature claims: confirmed — clarify README to state exact crypto/key-derivation and autodetect behavior.

  • Encryption: AESEncryptor uses AES-CBC (AES.MODE_CBC) with a 32‑byte key (dkLen=32) derived via PBKDF2 (SHA1) with configurable iterations; a 16‑byte value from get_random_bytes(16) is generated and reused as both the salt and the IV; output tag is "$aes-256-cbc/pbkdf2-sha1". (See android_sms_gateway/encryption.py)

  • HTTP backends: RequestsHttpClient and HttpxHttpClient (sync) and AiohttpAsyncHttpClient and HttpxAsyncHttpClient (async) are implemented; autodetection picks DEFAULT_CLIENT via get_client() in android_sms_gateway/http.py and android_sms_gateway/ahttp.py; pyproject.toml and README include the optional dependency extras and usage examples.

  • Action: update README lines to explicitly mention "PBKDF2‑SHA1 key derivation with configurable iterations" and that the IV is the randomly generated salt (or change implementation to use a separate IV), and clarify that httpx covers both sync+async and that the HTTP client is auto-detected.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e98ba39 and d910bb2.

📒 Files selected for processing (1)
  • README.md (4 hunks)
🔇 Additional comments (1)
README.md (1)

7-9: Badges now point to the correct repo — looks good.
Links and slugs resolve to android-sms-gateway/client-py.

Copy link

@coderabbitai coderabbitai bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
README.md (2)

95-100: Define missing extras in packaging metadata or update README

pyproject.toml's [project.optional-dependencies] contains only "requests" (pyproject.toml:38–40) while README advertises extras for requests, aiohttp, httpx and encryption (README.md:95–99,104–110). Add aiohttp = ["aiohttp"] and httpx = ["httpx"] (and encryption = ["..."]) to pyproject.toml or remove/update the README to match packaging.


268-279: README references missing http.AiohttpHttpClient — update example or add async client implementation.

android_sms_gateway/http.py defines RequestsHttpClient (lines 41–43) and HttpxHttpClient (lines 101–103); no AiohttpHttpClient class found. Update README.md (lines 268–279) to use an available async client or implement/export AiohttpHttpClient.

♻️ Duplicate comments (2)
README.md (2)

29-60: Fix broken Table of Contents anchors (remove leading hyphen).
GitHub heading slugs drop emojis and don’t start with “-”. Replace anchors like #-features with #features across the ToC.

Apply this diff:

- - [📱 SMS Gateway for Android™ Python API Client](#-sms-gateway-for-android-python-api-client)
-   - [📖 About The Project](#-about-the-project)
-   - [📚 Table of Contents](#-table-of-contents)
-   - [✨ Features](#-features)
-   - [⚙️ Requirements](#️-requirements)
-   - [📦 Installation](#-installation)
+ - [📱 SMS Gateway for Android™ Python API Client](#sms-gateway-for-android-python-api-client)
+   - [📖 About The Project](#about-the-project)
+   - [📚 Table of Contents](#table-of-contents)
+   - [✨ Features](#features)
+   - [⚙️ Requirements](#requirements)
+   - [📦 Installation](#installation)
@@
-   - [🚀 Quickstart](#-quickstart)
+   - [🚀 Quickstart](#quickstart)
@@
-   - [🤖 Client Guide](#-client-guide)
+   - [🤖 Client Guide](#client-guide)
@@
-   - [🌐 HTTP Clients](#-http-clients)
+   - [🌐 HTTP Clients](#http-clients)
@@
-   - [🔒 Security](#-security)
+   - [🔒 Security](#security)
@@
-   - [📚 API Reference](#-api-reference)
-   - [👥 Contributing](#-contributing)
+   - [📚 API Reference](#api-reference)
+   - [👥 Contributing](#contributing)
@@
-   - [📄 License](#-license)
-   - [🤝 Support](#-support)
+   - [📄 License](#license)
+   - [🤝 Support](#support)

283-283: Protocol reference alignment — resolved.
Async protocol now points to ahttp.AsyncHttpClient; this addresses the earlier mismatch. Thanks.

🧹 Nitpick comments (2)
README.md (2)

14-27: Use consistent product naming (“SMS Gate” vs “SMSGate”).
Line 16 uses “SMSGate”; elsewhere “SMS Gateway for Android”. Pick one and standardize (suggest: “SMS Gate” or “SMS Gateway for Android”) to avoid brand drift.


198-205: Clarify http param type for async client.
Table lists HttpClient, but AsyncAPIClient(http=...) expects an async protocol. Suggest “HttpClient | AsyncHttpClient (depends on client)”.

Apply this diff:

-| `http`      | `HttpClient` | Custom HTTP client  | Auto-detected                            |
+| `http`      | `HttpClient` (sync) / `AsyncHttpClient` (async) | Custom HTTP client  | Auto‑detected |
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d910bb2 and 381c9cd.

📒 Files selected for processing (1)
  • README.md (4 hunks)
🔇 Additional comments (5)
README.md (5)

7-9: Badges: repo links corrected — LGTM.
Updated Issues/Stars/Forks now point to android-sms-gateway/client-py.


174-190: Top-level export: Encryptor — confirmed.
android_sms_gateway/init.py does from .encryption import Encryptor and includes "Encryptor" in __all__, so README's from android_sms_gateway import Encryptor is correct.


124-169: Resolved: public imports & class names are correct.
android_sms_gateway/client.py defines APIClient (line 73) and AsyncAPIClient (line 181), and android_sms_gateway/init.py re-exports them — from android_sms_gateway import client, domain and client.APIClient / client.AsyncAPIClient are valid.


334-341: README: don't use --categories for package extras

pipenv's --categories targets Pipfile package groups (not setuptools "extras"); install extras inside the venv with pip (or use pipenv install '.[extras]'). (pipenv.pypa.io)

-# Create virtual environment
-pipenv install --dev --categories encryption,requests
-pipenv shell
+# Create virtual environment
+pipenv install --dev
+pipenv shell
+# Install package with desired extras inside the venv
+pip install "android-sms-gateway[encryption,requests]"

63-73: Encryption claim verified — README matches implementation.
Implementation uses AES.MODE_CBC with PBKDF2 (SHA1) for key derivation and Crypto.Util.Padding pad/unpad (PKCS#7); tests and format strings use "$aes-256-cbc/pbkdf2-sha1".
Locations: android_sms_gateway/encryption.py (imports AES, SHA1, PBKDF2; AES.new(..., AES.MODE_CBC, iv=...); pad/unpad) and tests/test_encryption.py (AESEncryptor usage + format string).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant