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 for gitlab subgroups and self-hosted instances #28

Merged
merged 6 commits into from
Feb 20, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/commits.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const MERGE_PATTERNS = [
/Merge pull request #(\d+) from .+\n\n(.+)/, // Regular GitHub merge
/^(.+) \(#(\d+)\)(?:$|\n\n)/, // Github squash merge
/Merged in .+ \(pull request #(\d+)\)\n\n(.+)/, // BitBucket merge
/Merge branch .+ into .+\n\n(.+)[\S\s]+See merge request !(\d+)/ // GitLab merge
/Merge branch .+ into .+\n\n(.+)[\S\s]+See merge request [^!]*!(\d+)/ // GitLab merge
]

export async function fetchCommits (remote, options) {
Expand Down Expand Up @@ -146,7 +146,7 @@ function getCommitLink (hash, remote) {
if (!remote) {
return null
}
if (remote.hostname === 'bitbucket.org') {
if (/bitbucket/.test(remote.hostname)) {
return `${remote.url}/commits/${hash}`
}
return `${remote.url}/commit/${hash}`
Expand All @@ -169,10 +169,10 @@ function getMergeLink (id, remote) {
if (!remote) {
return null
}
if (remote.hostname === 'bitbucket.org') {
if (/bitbucket/.test(remote.hostname)) {
return `${remote.url}/pull-requests/${id}`
}
if (remote.hostname === 'gitlab.com') {
if (/gitlab/.test(remote.hostname)) {
return `${remote.url}/merge_requests/${id}`
}
return `${remote.url}/pull/${id}`
Expand Down
2 changes: 1 addition & 1 deletion src/releases.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ function getCompareLink (from, to, remote) {
if (!remote) {
return null
}
if (remote.hostname === 'bitbucket.org') {
if (/bitbucket/.test(remote.hostname)) {
return `${remote.url}/compare/${to}%0D${from}`
}
return `${remote.url}/compare/${from}...${to}`
Expand Down
9 changes: 8 additions & 1 deletion src/remote.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,15 @@ export async function fetchRemote (name) {
const remote = parseRepoURL(remoteURL)
const protocol = remote.protocol === 'http:' ? 'http:' : 'https:'
const hostname = remote.hostname || remote.host
let repo = remote.repo

if (/gitlab/.test(hostname) && /\.git$/.test(remote.branch)) {
// Support gitlab subgroups
repo = `${remote.repo}/${remote.branch.replace(/\.git$/, '')}`
}

return {
hostname,
url: `${protocol}//${hostname}/${remote.repo}`
url: `${protocol}//${hostname}/${repo}`
}
}
13 changes: 13 additions & 0 deletions test/commits.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,19 @@ describe('getMerge', () => {
href: 'https://gitlab.com/user/repo/merge_requests/15007'
})
})

it('parses a merge for subgroups', () => {
const message = 'Merge branch \'branch\' into \'master\'\n\nMemoize GitLab logger to reduce open file descriptors\n\nCloses gitlab-ee#3664\n\nSee merge request user/repo/subgroup!15007'
const remote = {
hostname: 'gitlab.com',
url: 'https://gitlab.com/user/repo/subgroup'
}
expect(getMerge(message, remote)).to.deep.equal({
id: '15007',
message: 'Memoize GitLab logger to reduce open file descriptors',
href: 'https://gitlab.com/user/repo/subgroup/merge_requests/15007'
Copy link
Owner

@cookpete cookpete Feb 20, 2018

Choose a reason for hiding this comment

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

@a-ignatov-parc is this a valid/realistic URL for a subgroup merge request? I've updated the test, but I've never used gitlab subgroups. I want to keep the tests close to what is likely in the wild.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Here is a real link: https://gitlab.rambler.ru/unity/sites/wmj/merge_requests/193

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Btw thank for helping 😉

})
})
})

describe('BitBucket', () => {
Expand Down
7 changes: 7 additions & 0 deletions test/remote.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@ const TEST_DATA = [
remote: '[email protected]:user/repo.git',
expected: remotes.gitlab
},
{
remote: 'https://gitlab.com/user/repo#subgroup.git',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see something like https://gitlab.com/user/repo/subgroup.git in our subgroup project. Are you sure that this is correct form?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Real examples are https://gitlab.rambler.ru/unity/sites/wmj.git for https and [email protected]:unity/sites/wmj.git for ssh

Copy link
Owner

Choose a reason for hiding this comment

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

Ok good to know. I'll update the tests. I was only using # as that is what is used when you npm install a certain branch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you need some other urls to test just let me know 😉

expected: {
hostname: 'gitlab.com',
url: 'https://gitlab.com/user/repo/subgroup'
}
},
{
remote: 'https://bitbucket.org/user/repo',
expected: remotes.bitbucket
Expand Down