Leverage all-stars for author de-dupe and additional usernames

[nexdrew]

🌠 Initial stab to implement issue #6.

Basically adds a private getAllStar() function to util/package.js that all returned objects from getAuthor() and getMaintainers() are filtered through. The new method checks if the person is known to all-stars and merges its data with the current person object. This adds all properties and functions from the allStars.AllStar object (e.g. githubUser, npmUser, twitter, etc), which may have null or undefined values.

Added a single test to util/package.spec.js to cover the "found in all-stars" scenario. It uses mocked author data so that it's not dependent on actual data in all-stars, but unfortunately the mocking requires knowledge of how the data within all-stars is organized. If you think this is a problem, I'll probably need to add a mocking library (as a dev dependency) to mock out the all-stars methods instead.

This gets the job done, but suggestions for improvement are welcomed!

[stefanjudis]

Hey hey - thanks. :)

Could we maybe use an extend or assign function here to get rid of the if's?

As I see allStars is providing a summary function which could be easily used for that. :)

[nexdrew]

I will change this, but keep in mind that it will couple the object structure returned by credits to the object structure returned by all-stars. (And you may get more info than you want.) 😄

Here's an example:

{ name: 'Benjamin Coe',
  email: 'ben@npmjs.com',
  id: 'bcoe',
  npmUsers: [ 'bcoe' ],
  npmUser: 'bcoe',
  emails: [ 'ben@npmjs.com', 'bencoe@gmail.com' ],
  names: [ 'Benjamin Coe', 'Ben Coe', 'Benjamin E. Coe' ],
  githubUsers: [ 'bcoe' ],
  githubUser: 'bcoe',
  twitters: [ 'benjamincoe' ],
  twitter: 'benjamincoe',
  summary: [Function: summary],
  toString: [Function: toString],
  packages: 
   [ 'cliui',
     'os-locale',
     'y18n',
     'signal-exit',
     'nyc',
     'yargs',
     'wrap-ansi' ] }

[stefanjudis]

Looking good - only a few notes.

Great that you thought of the tests! 👍

[nexdrew]

Thanks for the feedback, it is much appreciated! I added commit 0cde7c0 to address your comments. Let me know if you find anything else. 😎

[stefanjudis]

Hey hey,

why don't you take the summary result here?

[nexdrew]

Ok, did you want the "long" or "short" summary? Long looks like:

Rod Vagg <r@va.gg> (npm: rvagg, GitHub: rvagg, Twitter: rvagg)

While short looks like:

Rod Vagg <r@va.gg>

BTW, did you see this comment about assigning? Are you ok returning all that stuff?

[stefanjudis]

Rod Vagg r@va.gg (npm: rvagg, GitHub: rvagg, Twitter: rvagg)

The representation thing is something related to the given reporter in credits-cli.
We should provide as much data as possible here without cluttering everything. :)

In a perfect world I'd like to have name, email, npm, github, twitter as strings ( or even array - I don't care too much about that one ).

BTW, did you see this comment about assigning? Are you ok returning all that stuff?

Jup - that's why I am asking for the summary call in all-stars. This will pretty much do the job, without exposing too much data, no?

[nexdrew]

Ok I'm confused.

My first approach explicitly added string properties npm, github, and twitter to the person object. But it was using a few if statements. You suggested using extend or assign instead.

My second approach used object-assign to implicitly override name and email and add the following properties to the person object:

• npmUser string
• githubUser string
• twitter string
• id string
• emails array
• names array
• npmUsers array
• githubUsers array
• twitters array
• summary function
• toString function

Which is probably a bit too much.

You mentioned you'd like the summary result, which could be represented as one of two strings (instead of a function). I asked which string you'd prefer, and I didn't understand your answer.

How many of the properties listed above would you like? Are you suggesting that the summary() function in all-stars return an object (instead of a string) with the following properties?

• name string
• email string
• npm string
• github string
• twitter string

And then merge this with the person object via object-assign? Something like this?

var allStar = allStars( person );
return allStar ? objectAssign( person, allStar.summary() ) : person;

[stefanjudis]

I'm really sorry about the confusion. :(

My first approach explicitly added string properties npm, github, and twitter to the person object. But it was using a few if statements. You suggested using extend or assign instead.

Using a lot of if/else statements in a row doesn't look good in my opinion and is not really extendable. Hopefully you agree there?

How many of the properties listed above would you like? Are you suggesting that the summary() function in all-stars return an object (instead of a string) with the following properties?

Aaah I see - here is the confusing coming from. I thought it is returning an object containing strings.
E.g.

{
  name : 'John Doe',
  email : 'john@doe.io',
  npm : 'johndoe',
  github : 'johndow',
  twitter : '@johndoe'
}

var allStar = allStars( person );
return allStar ? objectAssign( person, allStar.summary() ) : person;

This was pretty much what I had in mind, yes.

---

For all-stars I see 2 requests here ( if you agree ):

• provide a summary object
• improve documentation to make return clearer.

What do you think?

[nexdrew]

Ok, I think I can handle that. I will update. 😸 Thanks for working this out with me!

[stefanjudis]

👯 👍

[nexdrew]

Added commit 08efff7, which merges only the following properties from all-stars (each one is a string that may be null):

• name
• email
• npm
• github
• twitter

Let me know what you think. Thanks!

[stefanjudis]

Thanks for contributing. :)