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: change which reasons are selected for a non-redeemable policy #224

Merged
merged 1 commit into from
Jul 26, 2023
Merged

feat: change which reasons are selected for a non-redeemable policy #224

merged 1 commit into from
Jul 26, 2023

Conversation

brobro10000
Copy link
Contributor

Reorders the messaging from can_redeem to match figmas expected order of operation for error messaging return

pwnage101
pwnage101 reviewed Jul 26, 2023
View reviewed changes
Copy link
Contributor

@pwnage101 pwnage101 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

  • I think if the order of the reasons has an impact, we should document the rationale of the ordering in the docstring of this function.
  • Maybe a better commit message is "change which reasons are selected for a non-redeemable policy".

@brobro10000 brobro10000 changed the title feat: Reorder error messaging from can_redeem feat: change which reasons are selected for a non-redeemable policy Jul 26, 2023
pwnage101
pwnage101 approved these changes Jul 26, 2023
View reviewed changes
Copy link
Contributor

@pwnage101 pwnage101 left a comment

Choose a reason for hiding this comment

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

LGTM!

@brobro10000 brobro10000 merged commit fb9e347 into main Jul 26, 2023
2 checks passed
@brobro10000 brobro10000 deleted the hu/ent-7223-2 branch July 26, 2023 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pwnage101 pwnage101 pwnage101 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@brobro10000 @pwnage101