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

PR for Issue6: projectOwner & projectName is not set in .all-contributorsrc file. #80

Merged
merged 13 commits into from
May 29, 2018
Merged
4 changes: 3 additions & 1 deletion .all-contributorsrc
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,9 @@
"profile": "https://in.linkedin.com/in/mzubairahmed",
"contributions": [
"doc",
"bug"
"bug",
"code",
"test"
]
},
{
Expand Down
10 changes: 10 additions & 0 deletions src/util/__tests__/check.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import check from '../check'

test('Should throw error is no owner or name is present', () => {
expect(() => check('', 'cleave-markdown')).toThrow(
'Error! Project owner is not set in .all-contributorsrc',
)
expect(() => check('M-ZubairAhmed', '')).toThrow(
'Error! Project name is not set in .all-contributorsrc',
)
})
26 changes: 26 additions & 0 deletions src/util/__tests__/config-file.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,22 @@ import configFile from '../config-file'

const absentFile = './abc'
const expected = `Configuration file not found: ${absentFile}`
const configFileContent_NoOwner = {
projectOwner: '',
projectName: 'all-contributors-cli',
imageSize: 100,
commit: false,
contributorsPerLine: 6,
contributors: [],
}
const configFileContent_NoName = {
projectOwner: 'jfmengels',
projectName: '',
imageSize: 100,
commit: false,
contributorsPerLine: 6,
contributors: [],
}

test('Reading an absent configuration file throws a helpful error', () => {
expect(() => configFile.readConfig(absentFile)).toThrowError(expected)
Expand All @@ -13,3 +29,13 @@ test('Writing contributors in an absent configuration file throws a helpful erro
.catch(e => e)
expect(resolvedError.message).toBe(expected)
})

test('Should throw error and not allow editing config file if project name or owner is not set', () => {
const configPath = './.all-contributorsrc'
expect(() =>
configFile.writeConfig(configPath, configFileContent_NoOwner),
).toThrow(`Error! Project owner is not set in ${configPath}`)
expect(() =>
configFile.writeConfig(configPath, configFileContent_NoName),
).toThrow(`Error! Project name is not set in ${configPath}`)
})
54 changes: 54 additions & 0 deletions src/util/check.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
const pify = require('pify')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, it's been a little while, but why does this file exist? It doesn't appear to be in use anywhere in the project (other than tests)...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry but this is a node package isn't it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, I should have been more clear. I'm commenting on the file src/utils/check.js not pify.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry for being late.
It seems that this file was removed in this 152a1fe commit. I retested everything

const request = pify(require('request'))

function getNextLink(link) {
if (!link) {
return null
}

const nextLink = link.split(',').find(s => s.includes('rel="next"'))

if (!nextLink) {
return null
}

return nextLink.split(';')[0].slice(1, -1)
}

function getContributorsPage(url) {
return request
.get({
url,
headers: {
'User-Agent': 'request',
},
})
.then(res => {
let body, contributorsIds
try {
body = JSON.parse(res.body)
contributorsIds = body.map(contributor => contributor.login)
} catch (error) {
Error(error)
}
const nextLink = getNextLink(res.headers.link)
if (nextLink) {
return getContributorsPage(nextLink).then(nextContributors => {
return contributorsIds.concat(nextContributors)
})
}

return contributorsIds
})
}

module.exports = function getContributorsFromGithub(owner, name) {
if (!owner) {
throw new Error('Error! Project owner is not set in .all-contributorsrc')
}
if (!name) {
throw new Error('Error! Project name is not set in .all-contributorsrc ')
}
const url = `https://api.github.com/repos/${owner}/${name}/contributors?per_page=100`
return getContributorsPage(url)
}
6 changes: 6 additions & 0 deletions src/util/config-file.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ function readConfig(configPath) {
}

function writeConfig(configPath, content) {
if (!content.projectOwner) {
throw new Error(`Error! Project owner is not set in ${configPath}`)
}
if (!content.projectName) {
throw new Error(`Error! Project name is not set in ${configPath}`)
}
return pify(fs.writeFile)(configPath, `${JSON.stringify(content, null, 2)}\n`)
}

Expand Down