Skip to content

feat(clients): add Ruby client - #2

Open
cmozaiq-saas wants to merge 1 commit into
DZBuild-com:mainfrom
cmozaiq-saas:feat/ruby-client
Open

feat(clients): add Ruby client#2
cmozaiq-saas wants to merge 1 commit into
DZBuild-com:mainfrom
cmozaiq-saas:feat/ruby-client

Conversation

@cmozaiq-saas

Copy link
Copy Markdown

Adds clients/ruby/dzship.rb (stdlib only, Ruby 3.0+) and documents it in README and the integrating guide, with a full example and Rails line. Compatible with freeship v1.1

@ogyamada ogyamada left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, this is a good client. I ran it on Ruby 3.0 against the sandbox courier: lookups, wilaya_ship_code (68 resolves to 28), create/track/rates and the error envelope all behave. Two things before I merge.

  1. docs/integrating.md picked up an auto-format pass. The Node and Apps Script examples went from single to double quotes and the tables got padded, which is 49 of the 88 added lines and nothing to do with Ruby. The repo has no prettier config and the other docs keep the compact |---|---| tables. Could you revert those hunks and keep only the new Ruby section, the two table rows, and the raw HTTP line?

  2. credentials: is a required keyword and post always sends it. The other three clients made it optional in 1.1.0 so the sandbox courier works without a fake key. Right now DzShip.new(courier: "sandbox") raises missing keyword: :credentials, and credentials: nil sends "credentials": null, which the API rejects with a 400. Default it to nil and leave the key out of the body when it is nil or empty, the way the Python client does.

Nit, take it or leave it: the repo's commit subjects are plain sentences ("Add a Ruby client") rather than conventional prefixes.

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.

2 participants