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.

Sign up for GitHub

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

Jump to bottom

[feat] Add ability to add custom options #491

Merged
merged 4 commits into from
Dec 1, 2023
Merged

[feat] Add ability to add custom options #491

merged 4 commits into from
Dec 1, 2023

Conversation

nwithan8
Copy link
Contributor

@nwithan8 nwithan8 commented Jul 11, 2023 •
edited
Loading

Description

  • Adds ability for users to provide additional option key-value pairs in case an option is not available in the library

Testing

  • Add unit test to confirm that additional, user-defined options will be included in final serialized JSON payload

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)

@nwithan8 nwithan8 requested a review from a team July 11, 2023 22:42
@nwithan8 nwithan8 marked this pull request as draft July 12, 2023 21:11
@nwithan8 nwithan8 marked this pull request as ready for review July 12, 2023 21:49
@Justintime50
Copy link
Member

Do not introduce breaking changes, even if they are fixes. They require a major version bump. Leave them as they are, mark them with TODO comments, open a task, and fix them when we do the next major release. People rely and have coded around the inconsistencies. "Fixing" them will still break integrations.

@nwithan8 nwithan8 marked this pull request as draft July 13, 2023 16:48
@nwithan8 nwithan8 marked this pull request as ready for review November 30, 2023 23:17
dcaballeroc
dcaballeroc previously approved these changes Nov 30, 2023
View reviewed changes
@nwithan8
- Add ability for users to define their own options in case not expli…
a24f6ef 
…citly available in parameter set

- Add utilities to merge dictionaries, override parameter serialization process
- Add unit test to confirm adding custom options works
- Add missing options, fix incorrect option types
@nwithan8
- Update CHANGELOG
4474026
@nwithan8
- Linting
9b3431a
Justintime50
Justintime50 reviewed Dec 1, 2023
View reviewed changes
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.

I'm not sure we want to necessarily expose all of these. There are reasons why many options are undocumented

EasyPost/Models/API/Options.cs Outdated Show resolved Hide resolved
@nwithan8 nwithan8 marked this pull request as draft December 1, 2023 18:16
@nwithan8 nwithan8 changed the title [feat] Add missing options, ability to add custom options [feat] Add ability to add custom options Dec 1, 2023
@nwithan8 nwithan8 marked this pull request as ready for review December 1, 2023 20:28
@nwithan8 nwithan8 dismissed dcaballeroc’s stale review December 1, 2023 20:30

Scope change, need re-review

@nwithan8 nwithan8 merged commit b2dcb1f into master Dec 1, 2023
14 checks passed
@nwithan8 nwithan8 deleted the options branch December 1, 2023 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Justintime50 Justintime50 Justintime50 approved these changes

@dcaballeroc dcaballeroc dcaballeroc approved these changes

@jchen293 jchen293 Awaiting requested review from jchen293

@bnbry bnbry Awaiting requested review from bnbry

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nwithan8 @Justintime50 @dcaballeroc