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

Add revision links templates for GitHub and Azure DevOps services #6785

Merged
merged 1 commit into from Jun 30, 2019

Conversation

@pmiossec
Copy link
Member

commented Jun 12, 2019

Proposed changes

  • Being able to easily add revision links for GitHub and Azure DevOps services

Screenshots

Before

image

After

image

That create:

image

Result:

image

Test methodology

  • Manual and some Unit tests

Test environment(s)

  • Git Extensions 3.2.0
  • Build dee6de4cdfb281f0b42bb102ae9ea295ae032b98 (Dirty)
  • Git 2.21.0.windows.1
  • Microsoft Windows NT 10.0.17134.0
  • .NET Framework 4.7.3416.0
  • DPI 192dpi (200% scaling)

✒️ I contribute this code under The Developer Certificate of Origin.

@codecov

This comment has been minimized.

Copy link

commented Jun 12, 2019

Codecov Report

Merging #6785 into master will increase coverage by 0.14%.
The diff coverage is 86.91%.

@@            Coverage Diff             @@
##           master    #6785      +/-   ##
==========================================
+ Coverage   47.38%   47.53%   +0.14%     
==========================================
  Files         718      729      +11     
  Lines       53540    53751     +211     
  Branches     7051     7064      +13     
==========================================
+ Hits        25369    25549     +180     
- Misses      26784    26814      +30     
- Partials     1387     1388       +1
Flag Coverage Δ
#production 36.64% <82.5%> (+0.15%) ⬆️
#tests 97.69% <100%> (+0.01%) ⬆️
Copy link
Member

left a comment

I am not particular happy with the chosen design approach.

We are overloading RevisionLinksSettingsPage with the functionality that is orthogonal to the purpose of the control. This creates a maintenance overhead and unnecessary code coupling and reduces the code cohesion.

I propose we restructure the code in the following manner:

  • have an enum for available cloud providers:
    public enum CloudProviderKind 
    {
        None = 0,
        GitHub,
        AzureDevOps,
        // etc
    }
    
  • have a ICloudProviderExternalLinkDefinitionExtractor interface and cloud provider-specific implementations:
    public interface ICloudProviderExternalLinkDefinitionExtractor
    {
        bool TryExtract(string remoteUrl, out ExternalLinkDefinition definition)
    }
    
    public sealed class GitHubExternalLinkDefinitionExtractor : ICloudProviderExternalLinkDefinitionExtractor
    {
        public bool TryExtract(string remoteUrl, out ExternalLinkDefinition definition)
        {
            if (!TryExtractGitHubDataFromRemoteUrl(githubRemote.FetchUrl, out organizationName, out repoName))
            {
                return false;
            }
    
            var gitHubUrl = $"https://github.com/{organizationName}/{repoName}";
            definition = new ExternalLinkDefinition
            {
                Name = string.Format(_linkCodeToString.Text, serviceName),
                Enabled = true,
                SearchInParts = { ExternalLinkDefinition.RevisionPart.Message },
                SearchPattern = ".*",
                LinkFormats =
                {
                    new ExternalLinkFormat { Caption = string.Format(_linkViewCommitToString.Text, serviceName), Format = gitHubUrl + "/commit/%COMMIT_HASH%" },
                    new ExternalLinkFormat { Caption = string.Format(_linkViewProjectToString.Text, serviceName), Format = gitHubUrl }
                }
            };
    
            return true;
        }
    
        private static bool TryExtractGitHubDataFromRemoteUrl(string remoteUrl, out string owner, out string repository)
        {
            owner = null;
            repository = null;
    
            var m = MatchRegExes(remoteUrl, GitHubRegexes);
    
            if (m == null || !m.Success)
            {
                return false;
            }
    
            owner = m.Groups["owner"].Value;
            repository = m.Groups["repo"].Value.Replace(".git", "");
            return true;
        }
    }
    
  • have a ICloudProviderExternalLinkDefinitionExtractorFactory:
    public interface ICloudProviderExternalLinkDefinitionExtractorFactory
    {
        ICloudProviderExternalLinkDefinitionExtractor Get(CloudProviderKind cloudProviderKind)
    }
    
    public sealed class CloudProviderExternalLinkDefinitionExtractorFactory : ICloudProviderExternalLinkDefinitionExtractorFactory
    {
        public ICloudProviderExternalLinkDefinitionExtractor Get(CloudProviderKind cloudProviderKind)
        {
            switch (cloudProviderKind)
            {
                case CloudProviderKind.GitHub:
                {
                    return new GitHubExternalLinkDefinitionExtractor();
                }
            }
        }
    }
    

And then use ICloudProviderExternalLinkDefinitionExtractorFactory in RevisionLinksSettingsPage.

The benefits of the above approach:

  • SOLID implementations - each class does only one thing
  • adding, changing or removing a cloud provider should be straight forward and should require no changing at UI layer
  • tests should be simpler and easier to add and to manage

What do you think?

@pmiossec pmiossec force-pushed the pmiossec:revision_links_templates branch from a123f89 to 80e289d Jun 14, 2019
@pmiossec pmiossec force-pushed the pmiossec:revision_links_templates branch 3 times, most recently from 829cd4e to a3c71ab Jun 17, 2019
@pmiossec pmiossec changed the title Add revision links templates for GitHub and Azure DevOps services WIP: Add revision links templates for GitHub and Azure DevOps services Jun 19, 2019
@pmiossec pmiossec force-pushed the pmiossec:revision_links_templates branch 4 times, most recently from b495feb to 0892dd9 Jun 24, 2019
@pmiossec pmiossec changed the title WIP: Add revision links templates for GitHub and Azure DevOps services Add revision links templates for GitHub and Azure DevOps services Jun 25, 2019
+ Creation of GitHubRemoteParser & AzureDevOpsRemoteParser
@pmiossec pmiossec force-pushed the pmiossec:revision_links_templates branch from 0892dd9 to 606d1da Jun 25, 2019
@pmiossec

This comment has been minimized.

Copy link
Member Author

commented Jun 25, 2019

It seems good to me.

@pmiossec pmiossec requested a review from RussKie Jun 25, 2019
@RussKie RussKie merged commit f0ef891 into gitextensions:master Jun 30, 2019
3 checks passed
3 checks passed
CodeFactor No issues found.
Details
WIP Ready for review
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@RussKie RussKie added this to the 3.2.0 milestone Jun 30, 2019
@pmiossec pmiossec deleted the pmiossec:revision_links_templates branch Jul 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.