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

Support plurals when adding contributions e.g. docs (and doc) #15

Closed
jakebolam opened this issue Jan 11, 2019 · 9 comments
Closed

Support plurals when adding contributions e.g. docs (and doc) #15

jakebolam opened this issue Jan 11, 2019 · 9 comments
Labels
bug Something isn't working enhancement New feature or request good first issue Good for newcomers

Comments

@jakebolam
Copy link
Contributor

Is your feature request related to a problem? Please describe.
@AllContributorsBot please add jakebolam for docs, infra and code results in the additions of infra and code (not doc as well)

Describe the solution you'd like
The bot should be able to understand plurals. And add doc as well.

Describe alternatives you've considered

Additional context

@jakebolam jakebolam added enhancement New feature or request bug Something isn't working labels Jan 11, 2019
@Berkmann18
Copy link
Member

Berkmann18 commented Jan 11, 2019

I've looked at parse-comment and here are my thoughts:

1. How about validContributionTypes includes the plural of each item on the list

Possible solution

validContributionTypes.forEach(type => validContributionTypes.push(`${type}s`))

Pros

  1. It's One more line that will handle the valid types but in plural.
  2. Doesn't require more dependencies or numerous code to achieve this.

Cons

  1. It's not including the particular plural cases for: eventOrganizing (e.g.: eventsOrganizing), fundingFinding (e.g.: fundingsFinding), userTesting (e.g.: usersTesting).
  2. It's an additional O(n)/Θ(n) (where n = validContributionTypes.length) which will make the process slower as validContributionTypes grows.

2. Using RegExp

Possible solution

const TYPE_RE = /([a-z]+)([A-Z][a-z]+)/;
validContributionTypes
  .map(type => {
    let part = TYPE_RE.exec(type);
    if (part === null) return [type, `${type}s`]
    let [, start, end] = part;
    return [`${start}${end}`, `${start}s${end}`, `${start}${end}s`, `${start}s${end}s`];
  })
  .reduce((a, b) => a.concat(b), [])

Pros

  1. Native approach.
  2. Handles all singular/plural variations.

Cons

  1. Can be nasty and hard to maintain (especially as validContributionTypes changes).

3. Using what compromise offers

Possible solution

const contributions = doc
+       .forEach(type => contributions.push(type.toPlural()))
        .match('#Contribution')
        .data()
        .map(data => {
            // This removes whitespace, commas etc
            return data.normal
        })

Pros

  1. It utilises more the NLP library that this bot uses.
  2. It's less maintenance for this package when it comes to things like this.
  3. It can account for a wide array of plural forms that would be valid in English

Cons

  1. It might be tricky to get right (especially for people not familiar with compromise).

@jakebolam
Copy link
Contributor Author

@Berkmann18 thanks for looking into this. Really like the break down of options, so great!

I like your 3rd solution (continuing to use compromise). I think there is a learning curve to compromise but I'm hoping that it will allow the bot to expand into better parsing/understanding of user intentions.

I have the vision of the bot being able to recognise terms such as
@AllContributorsBot please add Berkmann18 for documentation, ideas
e.g.

  • doc could be: docs, documentation, documenting etc

Or even completly different wording.
@AllContributorsBot Berkmann18 has been great on the docs, could you add him as a contributor

Add maybe even additional languages

@allcontributors
Copy link
Contributor

@jakebolam

I'm not sure how to thanks

Basic usage: @@AllContributorsBot please add jakebolam for code, doc

@jakebolam
Copy link
Contributor Author

^ Need to figure out better ways of it responding to messages, not intended for it too (ticketed some stuff here #14)

@jakebolam jakebolam added the good first issue Good for newcomers label Jan 14, 2019
@jakebolam jakebolam moved this from To do to In progress in All Contributors Kanban Jan 15, 2019
@jakebolam
Copy link
Contributor Author

@Berkmann18 did you want to put a PR for this? As you did all the work :)

@Berkmann18
Copy link
Member

Berkmann18 commented Jan 15, 2019

@jakebolam I started playing around with the parse-comment module and compromise, trying to find a way to get the plural forms of the #Contributions included.

Berkmann18 referenced this issue in Berkmann18/all-contributors-bot Jan 15, 2019
Add the support for the plural forms of contributions (e.g: `docs`, `tests`, `plugins`, ...)
Berkmann18 referenced this issue in Berkmann18/all-contributors-bot Jan 15, 2019
Add the support for the plural forms of contributions (e.g: `docs`, `tests`, `plugins`, ...)
@Berkmann18
Copy link
Member

It turns out compromise doesn't handle that sort of thing without having a form of RegExp or array change and I was recommended to have the array with the plural forms in it before the .match(#Contribution) happens.

@Berkmann18
Copy link
Member

@jakebolam Don't forget to close this 😉.

@jakebolam
Copy link
Contributor Author

jakebolam commented Jan 19, 2019

Closing. Fixed in #53

All Contributors Kanban automation moved this from In progress to Done Jan 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request good first issue Good for newcomers
Projects
No open projects
Development

No branches or pull requests

2 participants