feat: add support for GitLab repositories #84

jgsmarques · 2018-02-19T14:29:02Z

What:

This change will introduce two new configurations:

repoType represents the type of repository and is a choice between github and gitlab (default is github)
repoHost represents where the repository is hosted, and it's default is https://github.com when repoType is github, and https://gitlab.com when repoType is gitlab.

Why:

These new configurations will allow to populate the contributors table with users that belong to registries other than https://github.com, which is currently the only one supported.

How:

By adding the two new configurations in the init step, and later using them while adding new contributors or generating the contributor list.

Checklist:

Documentation
Tests
Ready to be merged
Added myself to contributors table

The repoType is a choice between github and gitlab (default is github) and repoHost is a string which default to either 'https://github.com' or 'https://gitlab.com' depending on the answer given for 'repoType'

codecov-io · 2018-02-19T14:31:54Z

Codecov Report

Merging #84 into master will increase coverage by 0.75%.
The diff coverage is 62.96%.

@@            Coverage Diff             @@
##           master      #84      +/-   ##
==========================================
+ Coverage   52.79%   53.54%   +0.75%     
==========================================
  Files          18       19       +1     
  Lines         322      409      +87     
  Branches       44       66      +22     
==========================================
+ Hits          170      219      +49     
- Misses        135      161      +26     
- Partials       17       29      +12

Impacted Files	Coverage Δ
src/util/index.js	`100% <ø> (ø)`	⬆️
src/cli.js	`0% <0%> (ø)`	⬆️
src/init/prompt.js	`0% <0%> (ø)`	⬆️
src/contributors/prompt.js	`0% <0%> (ø)`	⬆️
src/contributors/index.js	`0% <0%> (ø)`	⬆️
src/util/contribution-types.js	`100% <100%> (ø)`	⬆️
src/contributors/add.js	`100% <100%> (ø)`	⬆️
src/util/config-file.js	`61.11% <25%> (-12.23%)`	⬇️
src/repo/gitlab.js	`39.39% <39.39%> (ø)`
src/repo/index.js	`83.78% <83.78%> (ø)`
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ab41caa...54304aa. Read the comment docs.

add methods to get config options, get users and get contributors add tests for most of the implemented code

fix both files to include 'repoType' and 'repoHost' in their options stub

…new repo api main changes include using methods that were fixed for github before, or passing extra parameters from the options object

…code all code that interacts with repositories is now under the repo folder, and therefore is deleted here

this allows for future call of these functions recursively if necessary

… method gitlab does not return the user login when the contributor's list is fetched. therefore, the name key is used to compare

the check command was broken because gitlab does not return the user login when fetching contributors. the name is now being used (because it is the only other data stored)

add 2 new items in the Configuration section: repoType and repoHost

jgsmarques · 2018-02-19T20:08:55Z

Hi @machour , do you want to take a look? I've done a bit of refactoring as well, in order to move all interactions with github to its own folder, to allow for a better abstraction for gitlab.

I made sure the existing unit tests kept working, and added my own tests to most of the code (but not all of it). I also tested it in a project I have on a private gitlab server, and it works like a charm :)

I'd appreciate your input in order to change whatever necessary

machour

@tigermarques this looks really promising!

I'll give it a deep review this afternoon. Could you meanwhile fix the whitespaces? It seems like you mixed tabs & spaces and some blocks are weirdly indented.

Please stick to 2 spaces for the indent.

jgsmarques · 2018-02-20T13:34:48Z

Sure I'll look into it. I'm used to having eslint shout at me about that. I'll change the indents to two spaces

jgsmarques · 2018-02-20T13:46:49Z

Fixed, and I've added two new rules to the eslintConfig section in package.json

{
    ...
    "no-mixed-spaces-and-tabs": "error",
    "indent": ["error", 2]
}

This enforces what you asked me to change so that eslint shouts at me. Do you want me to push this as well? For the time being, it is only on my own package.json

jgsmarques · 2018-02-20T14:10:53Z

@machour , I found these two npm packages that might be an option to replace our interactions with github and gitlab. What do you think? Maybe I could consider it for a future PR after you have decided whether to keep this feature or not.

jgsmarques

Changes implemented

jgsmarques · 2018-02-22T17:58:21Z

@machour, have you had a chance to look at the code?

machour · 2018-02-23T08:54:09Z

@tigermarques I will try my best to do that today (I need to create a gitlab account and test everything so this might take me a while).

Could you fix the little merge conflict?

github.js and gitlab.js now check for status codes >= 400 and print the respective error message

jgsmarques · 2018-02-23T10:11:30Z

Merge conflict fixed. Added the check for >= 400 status codes for both github and gitlab.

jgsmarques · 2018-02-23T10:12:40Z

If you want to use my user when testing in gitlab to add more contributors to a repo, fell free. It's jmarques.

machour · 2018-02-23T10:33:45Z

src/repo/gitlab.js

+        })
+        .then(newRes => {
+          const contributors = JSON.parse(newRes.body)
+					if (newRes.statusCode >= 400) {


Some tabs leftovers (here and the two following lines)

jgsmarques

Sorry about that. Fixed

machour

This is an outstanding work @tigermarques, congratulations!
Still something to review in the check command otherwise this looks really great! 🙌

machour · 2018-02-23T10:37:34Z

src/repo/gitlab.js

+					if (newRes.statusCode >= 400) {
+		        throw new Error(contributors.message)
+		      }
+          return contributors.map(item => item.name)


So sad that GitLab doesn't return the username.
I tested by changing my name in the .allcontributorsrc file to "Mehdi Acho".

Running the check command gives:

Missing contributors in .all-contributorsrc: Mehdi Achour

It also should have mentioned the unknown contributor found (Acho).
This happens around line 108 in src/cli.js

This comment got hidden as lines were changed. Don't forget to check it ;)

You're right I had missed it! So basically the error is that it detects the Gitlab user is not in the file, but does not detect that the contributor in the file is not in Gitlab. Is that it?

exactly "is not in the project's contributors as returned by Gitlab" to be precise

machour · 2018-02-23T10:38:38Z

src/cli.js

-  return util
-    .check(configData.projectOwner, configData.projectName)
+  return repo
+    .getContributors(configData.projectOwner, configData.projectName, configData.repoType, configData.repoHost)
    .then(ghContributors => {


ghContributors used to stand for GitHub Contributors.
Could you rename?

machour · 2018-02-23T10:39:03Z

src/cli.js

      )
-      const missingFromGithub = knownContributors.filter(login => {
+      const missingFromGithub = knownContributors.filter(key => {


Same here, this needs to be renamed

ghContributors was specific to GitHub, and therefore the variable is renamed to be more generic

machour · 2018-02-23T11:27:16Z

@tigermarques just making sure you've read this comment, which is blocking the approval:

jgsmarques · 2018-02-23T11:55:52Z

I've checked the comment now, and here is a test I produced:

create a repo and commit with my user
generate .all-contributorsrc
add myself as contributor with doc contribution
run check - does not detect anything as expected
manually edit my name in the .all-contributorsrc
run check:
- Detects one entry in Missing contributors in .all-contributorsrc as expected
- Does not detect any Unknown contributors because my contributions do not include neither code nor test
manually edit my contributions in the .all-contributorsrc to add code
run check - detects both differences now as expected

Can you check if by any chance your contributions are of types that don't include code or test. The code there is very explicit:

const missingFromRepo = knownContributors.filter(key => {
    return (
        !repoContributors.includes(key) &&
        (knownContributions[key].includes('code') ||
          knownContributions[key].includes('test'))
    )
})

machour · 2018-02-23T12:00:55Z

Gotcha, I didn't have a code or test contribution type. Added code and verified that it works like a charm.

reaaaaaaally minor thingy: when the repo haven't got any contribution yet (just created), if you run check you'll get 404 Not found printed without any explanations. This is because the gitlab api for contributors returns 404 in that case.

Might be better to catch it and output "no contributors found on GitLab" 🤔

…ound Message is "No contributors found on the GIthub repository

…ound Message is "No contributors found on the GitLab repository"

jgsmarques · 2018-02-23T12:09:41Z

Good catch. I was returning the default message that Gitlab sent.
For the sake of consistency, I added the same message for both github and gitlab (difference in the repo name only)

machour · 2018-02-23T12:11:50Z

Also, since this is a big change, and people are used to this lib working with GitHub only, it may be worth hinting about GitLab support.

Maybe change the sentence

This is a tool to help automate adding contributor acknowledgements according to the all-contributors specification.

to read

This is a tool to help automate adding contributor acknowledgements according to the all-contributors specification for your GitHub or GitLab repository.

under This solution in the README file

machour · 2018-02-23T14:13:24Z

Outstanding work @tigermarques 👏 👏 👏 👏

kentcdodds · 2018-03-12T16:17:30Z

src/repo/index.js

+    checkKey: 'login',
+    defaultHost: 'https://github.com',
+    linkToCommits: '<%= options.repoHost %>/<%= options.projectOwner %>/<%= options.projectName %>/commits?author=<%= contributor.login %>',
+    linkToIssues: '<%= options.repoHost %>/<%= options.projectOwner %>/<%= options.projectName %>/issues?q=author%3A<%= contributor.login %>',


Could we add || "https://github.com" here like we have for gitlab? It makes sense to me. Could you open up another PR for this @tigermarques?

Actually, don't bother. I'll go ahead and do it really quick :)

jgsmarques mentioned this pull request Feb 19, 2018

Support other sources other than Github #83

Closed

jgsmarques added 10 commits February 19, 2018 14:55

docs(README): add myself to contributors list

14f5874

feat(repo): add api to interact with repositories

945d79c

add methods to get config options, get users and get contributors add tests for most of the implemented code

test(generate): fix format-contribution-type and format-contributor

adf543b

fix both files to include 'repoType' and 'repoHost' in their options stub

refactor(contributors): adapt code that adds contributors to use the …

165eeec

…new repo api main changes include using methods that were fixed for github before, or passing extra parameters from the options object

refactor(cli): adapt cli to use the new api

f741884

refactor(util): adapt code to use the new repo api and remove unused …

e9d8295

…code all code that interacts with repositories is now under the repo folder, and therefore is deleted here

refactor(repo): declare the functions outside the module.exports

a13d060

this allows for future call of these functions recursively if necessary

feat(repo): add key to allow to compare users for gitlab in the check…

96b80d3

… method gitlab does not return the user login when the contributor's list is fetched. therefore, the name key is used to compare

fix(cli): fix the check command in gitlab based repositories

8f1bd55

the check command was broken because gitlab does not return the user login when fetching contributors. the name is now being used (because it is the only other data stored)

docs(README): add information about new parameters in the README file

99b2a99

add 2 new items in the Configuration section: repoType and repoHost

jgsmarques changed the title ~~[WIP] Support other sources other than Github.com~~ Support other sources other than Github.com Feb 19, 2018

machour requested changes Feb 20, 2018

View reviewed changes

refactor(): fix code style to use consistent indentation of 2 spaces

fdfdc52

jgsmarques commented Feb 20, 2018

View reviewed changes

docs(README): fix all-contributors user data

9bf96ff

chore(merge): merge branch with jfmengels/master

76ea5ea

github.js and gitlab.js now check for status codes >= 400 and print the respective error message

machour reviewed Feb 23, 2018

View reviewed changes

refactor(repo): fix tabs and whitespaces

07b4fc7

jgsmarques commented Feb 23, 2018

View reviewed changes

machour requested changes Feb 23, 2018

View reviewed changes

jgsmarques added 3 commits February 23, 2018 10:49

refactor(cli): rename ghContributors to repoContributors

7d2dad8

ghContributors was specific to GitHub, and therefore the variable is renamed to be more generic

refactor(cli): rename missingFromGithub to missingFromRepo

de73626

refactor(cli): rename GitHub to Repository in the CLI help commands

9fdfda5

jgsmarques added 2 commits February 23, 2018 12:07

refactor(github): add custom error message when no contributors are f…

7a906e6

…ound Message is "No contributors found on the GIthub repository

refactor(gitlab): add custom error message when no contributors are f…

3ac2d0f

…ound Message is "No contributors found on the GitLab repository"

machour changed the title ~~Support other sources other than Github.com~~ feat: add support for GitLab repositories Feb 23, 2018

docs(README): add more detail about GitLab support

54304aa

machour approved these changes Feb 23, 2018

View reviewed changes

machour merged commit 152a1fe into all-contributors:master Feb 23, 2018

kentcdodds reviewed Mar 12, 2018

View reviewed changes

kentcdodds mentioned this pull request Mar 12, 2018

Add docs for default keydown handlers downshift-js/downshift#370

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add support for GitLab repositories #84

feat: add support for GitLab repositories #84

jgsmarques commented Feb 19, 2018 •

edited

Loading

codecov-io commented Feb 19, 2018 •

edited

Loading

jgsmarques commented Feb 19, 2018 •

edited

Loading

machour left a comment

jgsmarques commented Feb 20, 2018

jgsmarques commented Feb 20, 2018

jgsmarques commented Feb 20, 2018

jgsmarques left a comment

jgsmarques commented Feb 22, 2018

machour commented Feb 23, 2018

jgsmarques commented Feb 23, 2018

jgsmarques commented Feb 23, 2018 •

edited

Loading

machour Feb 23, 2018 •

edited

Loading

jgsmarques left a comment

machour left a comment

machour Feb 23, 2018

machour Feb 23, 2018

jgsmarques Feb 23, 2018

machour Feb 23, 2018

machour Feb 23, 2018

machour Feb 23, 2018

machour commented Feb 23, 2018

jgsmarques commented Feb 23, 2018

machour commented Feb 23, 2018

jgsmarques commented Feb 23, 2018

machour commented Feb 23, 2018

machour commented Feb 23, 2018

kentcdodds Mar 12, 2018

kentcdodds Mar 12, 2018

feat: add support for GitLab repositories #84

feat: add support for GitLab repositories #84

Conversation

jgsmarques commented Feb 19, 2018 • edited Loading

codecov-io commented Feb 19, 2018 • edited Loading

Codecov Report

jgsmarques commented Feb 19, 2018 • edited Loading

machour left a comment

Choose a reason for hiding this comment

jgsmarques commented Feb 20, 2018

jgsmarques commented Feb 20, 2018

jgsmarques commented Feb 20, 2018

jgsmarques left a comment

Choose a reason for hiding this comment

jgsmarques commented Feb 22, 2018

machour commented Feb 23, 2018

jgsmarques commented Feb 23, 2018

jgsmarques commented Feb 23, 2018 • edited Loading

machour Feb 23, 2018 • edited Loading

Choose a reason for hiding this comment

jgsmarques left a comment

Choose a reason for hiding this comment

machour 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

machour commented Feb 23, 2018

jgsmarques commented Feb 23, 2018

machour commented Feb 23, 2018

jgsmarques commented Feb 23, 2018

machour commented Feb 23, 2018

machour commented Feb 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgsmarques commented Feb 19, 2018 •

edited

Loading

codecov-io commented Feb 19, 2018 •

edited

Loading

jgsmarques commented Feb 19, 2018 •

edited

Loading

jgsmarques commented Feb 23, 2018 •

edited

Loading

machour Feb 23, 2018 •

edited

Loading