Skip to content

Conversation

jchen293
Copy link
Contributor

Description

Add content for the V6 major bump in the upgrade guide
Move the ExternalApiError from the general to API folder

Testing

N/A

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 marked this pull request as ready for review November 29, 2022 22:15
@jchen293 jchen293 requested a review from a team November 29, 2022 22:15
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'd argue we need the following items in the upgrade guide too:

  • Changes the type result of Event from EasyPostResource to Map<String, Object>
  • Changes the type of Insurance Amount from Float to String
  • Removes all the setters of each object (we say setters are available via lombok; however, I don't think we added the decorator meaning users can't access them. In the off chance someone was previously, we need to call it out that they can't any longer)
  • Rename some getters
    • Pickup class: getPickoutRates() -> getPickupRates()
    • PaymentMethod class: getPrimaryPaymentMethodObject() -> getPrimaryPaymentMethod()
    • PaymentMethod class: getSecondaryPaymentMethodObject() -> getSecondaryPaymentMethod()

The point of the upgrade guide is to callout anything that will require a user to change their code and provide guidance and examples on how to do it to ease the process of moving from one major version to another. We don't need to call out additions here (the CHANGELOG can handle that), but the upgrade guide should include anything substantial that will require an engineer to make a tweak in a codebase downstream from us.

nwithan8
nwithan8 previously approved these changes Nov 30, 2022
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.

Various small nits to fix grammar/spelling

Justintime50
Justintime50 previously approved these changes Nov 30, 2022
@jchen293 jchen293 merged commit 68e75c5 into master Nov 30, 2022
@jchen293 jchen293 deleted the upgrade_guide branch November 30, 2022 17:03
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.

3 participants