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

Change desired_delivery_date to str instead of key-word args #340

Merged
merged 1 commit into from
Jul 11, 2024

Conversation

jchen293
Copy link
Member

@jchen293 jchen293 commented Jul 11, 2024

Description

Change desired_delivery_date to str instead of key-word args in Shipment.recommend_ship_date function

Testing

We should expect users to input a string and not a dict for the Shipment.recommend_ship_date function, this matches retrieve_estimated_delivery_date function

Pull Request Type

Please select the option(s) that are relevant to this PR.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Improvement (fixing a typo, updating readme, renaming a variable name, etc)

@jchen293 jchen293 requested a review from a team July 11, 2024 17:37
Copy link
Member

@Justintime50 Justintime50 left a comment

Choose a reason for hiding this comment

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

Are we doing this for all the libs? What if a param gets added?

@jchen293
Copy link
Member Author

Are we doing this for all the libs? What if a param gets added?

We haven't implemented this in the other six libraries yet, so we can make a decision here. This change matches retrieve_estimated_delivery_date above, which expects a string, an inconsistency I just discovered. The precision_shipping endpoint expects desired_delivery_date which should always be a single date parameter, so I don't foresee the API making a breaking change. Since both shipment smartrate endpoints are somewhat similar, this is more of a question of whether we want to wrap the parameter for users in the client libs.

@jchen293 jchen293 merged commit 05e9869 into master Jul 11, 2024
8 checks passed
@jchen293 jchen293 deleted the fix_recommend_ship_date_parameter branch July 11, 2024 18:36
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