methods to make fake pizza data #90

lauraannwilliams · 2017-05-08T03:05:33Z

In addition to generating a fake pizza company (mostly drawn from Pop Culture), this will generate a variety of international toppings, combos, and styles.

I've updated:

USAGE.md docs
CHANGELOG.md

…les and toppings

tobyhinloopen

I left some comments about the code changes

tobyhinloopen · 2017-05-08T08:24:36Z

lib/faker/pizza.ex

+  @doc """
+  Returns a size string
+  """
+  @spec combo() :: Strint.t


I think this is a typo

tobyhinloopen · 2017-05-08T08:24:44Z

lib/faker/pizza.ex

+  @doc """
+  Returns a size string
+  """
+  @spec size() :: Strint.t


I think this is a typo

I got what you were referring to for most of them, but not sure what the typo is here?

Strint.t is here

tobyhinloopen · 2017-05-08T08:24:48Z

lib/faker/pizza.ex

+  @doc """
+  Returns a inches string
+  """
+  @spec inches() :: Strint.t


I think this is a typo

tobyhinloopen · 2017-05-08T08:25:11Z

mix.lock

+%{"bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], []},
+  "credo": {:hex, :credo, "0.7.3", "9827ab04002186af1aec014a811839a06f72aaae6cd5eed3919b248c8767dbf3", [:mix], [{:bunt, "~> 0.2.0", [hex: :bunt, optional: false]}]},
+  "earmark": {:hex, :earmark, "1.2.0", "bf1ce17aea43ab62f6943b97bd6e3dc032ce45d4f787504e3adf738e54b42f3a", [:mix], []},
+  "ex_doc": {:hex, :ex_doc, "0.15.1", "d5f9d588fd802152516fccfdb96d6073753f77314fcfee892b15b6724ca0d596", [:mix], [{:earmark, "~> 1.1", [hex: :earmark, optional: false]}]}}


Is this relevant to the PR? Why is this changed?

I assume these were out of date on the repo. I can remove them if @igas requests :)

@lauraannwilliams I don't think so, pls update your hex by running mix local.hex and try to run it again.

I didn't realize my hex was out of date. Thanks :)

tobyhinloopen · 2017-05-08T08:26:53Z

lib/faker/pizza.ex

+  @doc """
+  Converts a list to a string, with "and" before the last item
+  """
+  @spec add_and(list) :: [char]


You're returning a String.t, right? Not a char list?

tobyhinloopen · 2017-05-08T08:26:59Z

lib/faker/pizza.ex

+  @doc """
+  Returns a specified number of toppings as a string, with an "and" before the last topping
+  """
+  @spec toppings_and(integer) :: [char]


You're returning a String.t, right? Not a char list?

tobyhinloopen · 2017-05-08T08:35:27Z

First of all, thank you for your PR. Who doesn't like Pizza, right?

However, and I don't know how to say this without being some kind of party pooper: A couple of "fun" based Faker modules is fine, but I don't think we want to add too many. We already have Pokemon, Beer and Superhero. I highly doubt anyone actually uses these, while they do add extra code to maintain.

I'm curious to know how @igas feels about adding more "fun" Faker data.

lauraannwilliams · 2017-05-08T16:16:51Z

I really appreciate you catching my various typos - I'll clean that up today.

As for the "fun" aspect, I built this one because I had a real use case. I'm working on code for a decision making process. We use 'Pizza' related examples all the time because they're very accessible to new users. It lets them focus on the technology, instead of the data.

Not that I mind 'Fun' aspects either. If you look at https://github.com/stympy/faker you'll see a lot of fun ones, that I would use - Like Star Wars, Game of Thrones, etc. If I'm populating a demo with 'fake' comments, it's nice to have usernames that make it clear it's sample data.

tobyhinloopen

Sounds like a reasonable use-case to me. Left some additional comments about the Strint.t typos.

Once the typos are fixed, its LGTM from me.

tobyhinloopen · 2017-05-08T19:27:41Z

lib/faker/pizza.ex

+  @doc """
+  Returns a combo string
+  """
+  @spec combo() :: Strint.t


Another Strint

tobyhinloopen · 2017-05-08T19:27:52Z

lib/faker/pizza.ex

+  @doc """
+  Returns an inches string
+  """
+  @spec inches() :: Strint.t


Another Strint

tobyhinloopen

LGTM

lauraannwilliams · 2017-05-10T16:36:03Z

Thanks :)

igas · 2017-05-11T09:19:02Z

@tobyhinloopen I don't mind fun modules, I think it's our responsibility to make it easier to add and maintain. In our projects we use quite a lot of fun names for different demo things.

igas · 2017-05-11T09:20:08Z

CHANGELOG.md

@@ -9,6 +9,7 @@ Change log itself follows [Keep a CHANGELOG](http://keepachangelog.com) format.
 ## Unreleased

 ### Added
+* `Faker.Pizza` [@lauraannwilliams][]


This is just label in markdown, you also need to add URL to your page at the end of changelog.

igas · 2017-05-11T09:21:13Z

USAGE.md

+
+```elixir
+
+Faker.Pizza.pizzas(%Range{first: 2, last: 3}) => ["Medium Thai Chicken", "26\" with Eel and Salami", "Large with Squid, Duck, Meatballs, and Classic Tomato Sauce"]


we can use 2..3

igas · 2017-05-11T09:21:56Z

USAGE.md

+
+Faker.Pizza.pizzas(%Range{first: 2, last: 3}) => ["Medium Thai Chicken", "26\" with Eel and Salami", "Large with Squid, Duck, Meatballs, and Classic Tomato Sauce"]
+
+Faker.Pizza.pizzas(2) => ["30\" with Alfredo Sauce, Scallops, Lactose Free Cheese, and Cherry Tomatoes", "Family Deep Fried Pizza Double Dutch"]


please change => to # =>

igas · 2017-05-11T09:53:11Z

lib/faker/pizza.ex

+  If no range is specified it defaults to 2..5
+  """
+  @spec toppings_and(Range.t) :: String.t
+  def toppings_and(range \\ %Range{first: 2, last: 5})


maybe call this one toppings and current toppings rename to list_of_toppings. I'm not sure suffix and looks good in function name. What do you think @tobyhinloopen ?

toppings_sentence combined with renaming add_and to to_sentence. Inspired by rails' to_sentence

igas · 2017-05-11T10:05:19Z

lib/faker/pizza.ex

+  Returns a list with a random number of pizzas (in between the specified
+  range)
+  If no range is specified it defaults to 2..5
+  """


If I remember correctly, this documentation will be overridden by next one because function have the same arity, it's need to be rephrased and grouped into one.

I was emulating the style in the Lorem module. I've updated it to single documentation block

…mments

jessejanderson · 2017-05-12T17:43:34Z

Some variety of "fun" modules are also nice to have when you need to generate something that there isn't a perfect match for. I needed "random event names" and ended up using some combo of the Superhero and Beer modules to get something that semi resembled an event name. ;)

lauraannwilliams · 2017-05-19T17:36:52Z

Please let me know if there's anything else you need from me :)

igas · 2017-05-21T03:03:16Z

Hi @lauraannwilliams it looks great. Only thing unrelated to your code I want to discuss with you and @tobyhinloopen. We added to_sentence/1 which is awesome and generic, what do you think if we move it to Faker.Util or Faker to use everywhere. So we can drop toppings_sentence/1 in this case. And use this new function directly in pizza/1.

tobyhinloopen · 2017-05-21T06:32:32Z

https://github.com/igas/faker/pull/85/files#diff-eaf63e239fc3f2d2aca36b4fb9b9ef4aR50

lauraannwilliams · 2017-05-21T19:30:18Z

Sounds good to me.

igas · 2017-06-27T08:36:21Z

I think all looks good, except toppings_sentence and to_sentence. As discussed earlier it'll be to many APIs, if we'll start adding it to every function that return list, I think it should be on consumer level through Util, and the ones that here already using it should also go through Util. So lets drop this 2 and merge it.

lauraannwilliams · 2017-06-30T18:33:09Z

I was waiting for the other pull request, didn't realize you were waiting on me!

I'll yank out the to_sentence this weekend.

igas · 2017-07-01T02:01:17Z

Thank you @lauraannwilliams

lauraannwilliams · 2017-07-02T02:14:47Z

Updates complete :)

lauraannwilliams · 2017-07-11T17:55:11Z

I see this is failing a credo check, i'll update this week.

igas · 2017-07-12T11:30:17Z

@lauraannwilliams it is actually me, master have similar problem. It would be awesome if you'll find the cause. I couldn't find the time to come back to this problem. If you, me or @tobyhinloopen will not find the cause of types issue, maybe we need to turn it off, or relax versions on CI.

igas · 2017-07-16T12:29:18Z

❤️ 💚 💙 💛 💜

lauraannwilliams · 2017-07-17T02:06:34Z

yay!

Laura Williams added 2 commits May 7, 2017 19:59

methods to make fake pizza data, with an variety of international sty…

2064748

…les and toppings

Update Changelog

239f89b

lauraannwilliams mentioned this pull request May 8, 2017

Pizza Data (PR Added) #91

Closed

tobyhinloopen reviewed May 8, 2017

View reviewed changes

fix some typos as per PR comments

93d012a

tobyhinloopen reviewed May 8, 2017

View reviewed changes

fix cut and paste typos

aba0f4a

tobyhinloopen approved these changes May 10, 2017

View reviewed changes

igas reviewed May 11, 2017

View reviewed changes

Tidy documentation, fix changelog link, rename functions as per PR co…

18847d8

…mments

igas force-pushed the master branch from c09b247 to e6a874e Compare June 9, 2017 08:48

Laura Williams and others added 2 commits July 1, 2017 19:03

Merge remote-tracking branch 'upstream/master'

de42a44

use Util to_sentence

82d1fc8

fix typo

4d60784

fix credo errors

e2dd24f

igas merged commit 1740cf2 into elixirs:master Jul 16, 2017

dependabot-preview bot mentioned this pull request Jul 23, 2019

Bump earmark from 1.2.6 to 1.3.3 MacGregor-Archive/faker#19

Closed


		```elixir

		Faker.Pizza.pizzas(%Range{first: 2, last: 3}) => ["Medium Thai Chicken", "26\" with Eel and Salami", "Large with Squid, Duck, Meatballs, and Classic Tomato Sauce"]


		Faker.Pizza.pizzas(%Range{first: 2, last: 3}) => ["Medium Thai Chicken", "26\" with Eel and Salami", "Large with Squid, Duck, Meatballs, and Classic Tomato Sauce"]

		Faker.Pizza.pizzas(2) => ["30\" with Alfredo Sauce, Scallops, Lactose Free Cheese, and Cherry Tomatoes", "Family Deep Fried Pizza Double Dutch"]

methods to make fake pizza data #90

methods to make fake pizza data #90

Conversation

lauraannwilliams commented May 8, 2017

tobyhinloopen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

igas May 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobyhinloopen commented May 8, 2017

lauraannwilliams commented May 8, 2017 • edited Loading

tobyhinloopen left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobyhinloopen left a comment

Choose a reason for hiding this comment

lauraannwilliams commented May 10, 2017

igas commented May 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jessejanderson commented May 12, 2017

lauraannwilliams commented May 19, 2017

igas commented May 21, 2017

tobyhinloopen commented May 21, 2017

lauraannwilliams commented May 21, 2017

igas commented Jun 27, 2017

lauraannwilliams commented Jun 30, 2017

igas commented Jul 1, 2017

lauraannwilliams commented Jul 2, 2017

lauraannwilliams commented Jul 11, 2017

igas commented Jul 12, 2017

igas commented Jul 16, 2017

lauraannwilliams commented Jul 17, 2017

igas May 11, 2017 •

edited

Loading

lauraannwilliams commented May 8, 2017 •

edited

Loading

tobyhinloopen left a comment •

edited

Loading