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

don't use apply method in gift code generator #2643

Merged
merged 2 commits into from Aug 4, 2020
Merged

Conversation

johnduffell
Copy link
Member

@johnduffell johnduffell commented Aug 3, 2020

Why are you doing this?

Following a discussion on RR dev channel, we agreed that the apply method behaviour is non-obvious, especially for client side engineers.

This PR removes the places we use it for normal code.

We also use it for case class smart constructors.
We probably prefer "parse" but you need more tricks (until we update to 2.13 - scala/scala#7702 ) to get rid of the built in apply and not defeat the point of the constructor. Some options:
image
or
image
or
image
or
keep it as apply.

Copy link
Member

@tomrf1 tomrf1 left a comment

Choose a reason for hiding this comment

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

looks good!

@johnduffell johnduffell merged commit af0d87e into master Aug 4, 2020
@johnduffell johnduffell deleted the jd-giftcode-tidyup branch August 4, 2020 13:03
@prout-bot
Copy link

Seen on PROD (merged by @johnduffell 13 minutes and 9 seconds ago)

Sentry Release: support-client-side, support

@prout-bot
Copy link

✅ Testing in PROD passed! Details

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants