[utils] Format all git domains, not only github.com

[AndrewChubatiuk]

What and why?

Enabled formatting of all git remotes, not only github.com to fix datadog serverless plugin tags, while using in projects with repos that are not hosted on github

How?

updated regular expression to replace fix remote urls with any domain name

[Drarig29]

Hi, thanks for your contribution!

Can you test case(s) for what you changed, and ensure the unit tests are passing?

You can find the existing tests here:

datadog-ci/src/helpers/__tests__/utils.test.ts

Lines 374 to 384 in 19a9e0c

	describe('filterAndFormatGithubRemote', () => {
	test('git remotes get formatted correctly', async () => {
	expect(ciUtils.filterAndFormatGithubRemote('https:/datadog/test.git')).toEqual(
	'github.com/datadog/test.git'
	)
	expect(ciUtils.filterAndFormatGithubRemote('[email protected]:datadog/test.git')).toEqual(
	'github.com/datadog/test.git'
	)
	expect(ciUtils.filterAndFormatGithubRemote('github.com/datadog/test.git')).toEqual('github.com/datadog/test.git')
	})
	})

[AndrewChubatiuk]

@Drarig29 done

[Drarig29]

Why would you remove the port? Do you have an example of scenario?

[AndrewChubatiuk]

this function is being used by serverless-datadog-plugin and colon symbol is a delimiter for key:value pair, had to remove port as well to fix

[duncanista]

If we are formatting all git domains, shouldn't we also change the function name (and jsdocs) to be something different than filterAndFormatGithubRemote? Maybe just filterAndFormatGitRemote? Just a nit, but would be better to keep consistency.

[AndrewChubatiuk]

I've renamed it, but this function is exported, so some apps or libs, that depend on it can become broken

[Drarig29]

@AndrewChubatiuk good point!

Can you add the original filterAndFormatGithubRemote() function back (with its old implementation and unit tests) and mark it as deprecated like that function?

Thanks in advance, and sorry for the delay!

[AndrewChubatiuk]

added filterAndFormatGithubRemote() as alias to filterAndFormatGitRemote() and marked as deprecated

[Drarig29]

Nice, indeed the unit test did not change, so the new implementation did not change in behavior for the old supported cases.

[duncanista]

LGTM – left a comment!

[Drarig29]

@AndrewChubatiuk I triggered the CI for your latest commit, and the lint phase fails. Running yarn format should fix it!

[Drarig29]

LGTM, thanks for the contribution!

[rpelliard]

Hello @AndrewChubatiuk !

Enabled formatting of all git remotes, not only github.com to fix datadog serverless plugin tags, while using in projects with repos that are not hosted on github

Out of curiosity, are you encountering an issue with the current version ? For context, this normalization for GitHub URLs was introduced to circumvent some limitations around some old tracer versions that don't support the URL scheme in tags, but it shouldn't be necessary in most cases now.

For GitHub repositories, the git@ and https:// repository URLs are predictable / interchangable, so we don't lose any information by stripping the scheme. However, this is not the case for all Git repo hosting providers, so I'd like to make sure we fully understand your issue before merging this change - Thanks !

[AndrewChubatiuk]

@rpelliard this function is being used by serverless datadog plugin and it throws exceptions when tries to add labels generated from git: when using non-github git@ origin it fails cause labelname:git@domain:org/repo has more than one colon symbol

[rpelliard]

@AndrewChubatiuk What version of serverless-plugin-datadog, and which language are you using ? The latest one should only bundle versions of the tracing libraries that support the double columns in the tags