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

Filter backings from recommended projects #74

Merged
merged 2 commits into from
Feb 7, 2017
Merged

Conversation

kyeah
Copy link
Contributor

@kyeah kyeah commented Feb 3, 2017

hey it's me again, just filtering backings, don't mind me

in case you forgot:

We filter user's backed projects from recommendations on web, and we also filter them in the native Thanks activity. Filtering them here will homogenize the backer experience across our platforms and stop people from asking jeff what's up with that rec engine.

The Thanks activity also filters backings from staff picks, so that's a possibility here.

cc. @kickstarter/native-squad

filters.append(.defaults |> DiscoveryParams.lens.recommended .~ true)
filters.append(.defaults
|> DiscoveryParams.lens.recommended .~ true
<> DiscoveryParams.lens.backed .~ false)
Copy link
Contributor

@luoser luoser Feb 3, 2017

Choose a reason for hiding this comment

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

nice! our general code style when using multiple lenses within () is to drop it to its own new section, i.e.

filters.append(
  .defaults
    |> DiscoveryParams.lens.recommended .~ true
    |> DiscoveryParams.lens.backed .~ false
)

Also we continue to use the pipe |> here since we are piping values into the .defaults. I think <> is more about composing, e.g. in our styles, but I don't fully remember and my xcode isn't opening right now to find the operator

[edit] here it is

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah cool! I forgot where to find these ops and didn't know how to use 'em, but this makes sense 🙇

@kyeah kyeah merged commit f5df71a into kickstarter:master Feb 7, 2017
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