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

Make branch name comparison case insensitive #2261

Merged
merged 3 commits into from
May 11, 2020

Conversation

asbjornu
Copy link
Member

@asbjornu asbjornu commented May 8, 2020

Description

This PR introduces the IsEquivalentTo() extension method to System.String and uses it to make branch name comparison case insensitive.

Related Issue

Fixes #864.

Motivation and Context

Working on Windows with a case insensitive file system and running GitVersion in a CI/CD pipeline on Linux, the case sensitivity of Linux often cause problems that aren't discovered on Windows. This change should remedy that discrepancy.

How Has This Been Tested?

I added a poorly constructed test to provoke the issue. Not optimal, but better than nothing. 🤷‍♂️

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@asbjornu asbjornu changed the title Make comparison of case insensitive Make branch name comparison case insensitive May 8, 2020
@asbjornu asbjornu force-pushed the feature/case-insensitive-equals branch from c8f17be to 28e9f9e Compare May 8, 2020 10:29
@asbjornu asbjornu force-pushed the feature/case-insensitive-equals branch from 28e9f9e to e85f601 Compare May 8, 2020 10:36
@arturcic arturcic marked this pull request as draft May 8, 2020 10:50
@asbjornu asbjornu force-pushed the feature/case-insensitive-equals branch 2 times, most recently from 5689de4 to 22ee2bc Compare May 11, 2020 14:05
@asbjornu asbjornu force-pushed the feature/case-insensitive-equals branch 3 times, most recently from f84b065 to a79fc31 Compare May 11, 2020 15:18
Add IGitRepositoryCommands interface and default implementation as a mockable and testable wrapper around the static Commands class.
@asbjornu asbjornu force-pushed the feature/case-insensitive-equals branch from a79fc31 to f3a50b6 Compare May 11, 2020 15:19
@asbjornu asbjornu requested a review from arturcic May 11, 2020 15:27
@asbjornu asbjornu marked this pull request as ready for review May 11, 2020 15:27
@asbjornu
Copy link
Member Author

I'm not happy how the test turned out, but it's better than nothing I guess.

Copy link
Member

@arturcic arturcic left a comment

Choose a reason for hiding this comment

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

I like the way you abstracted the Commands and included as part of GitRepository.

In version 6.0 I would want to move the LibGit2Sharp code into a separate class, maybe we can move it outside of the Core. And if have luck we might have 2 implementations, one with LibGit2Sharp and other with let's say Octokit

@asbjornu
Copy link
Member Author

Yes, that would be ideal, @arturcic! A dependency-free Core would be awesome.

@arturcic
Copy link
Member

As part of part di4 I tried to move the libgit2sharp code to IRepositoryMetadataProvider and RepositoryExtensions and added as injectable IRepository, now in the next iteration we need to create our version of IRepository interface, Branch, Commit classes, and then we can reduce the dependency of the code on Libgit2Sharp

@arturcic
Copy link
Member

When the build succeeds feel free to merge it yourself. Just add the milestone to the issue you're fixing so it is included in the release notes

@asbjornu asbjornu added this to the 5.3.x milestone May 11, 2020
@asbjornu asbjornu merged commit ba7992f into GitTools:master May 11, 2020
@asbjornu asbjornu deleted the feature/case-insensitive-equals branch May 11, 2020 15:50
@arturcic arturcic modified the milestones: 5.3.x, 5.3.3 May 12, 2020
@mcascone
Copy link

Please see my comment on the original issue: I am seeing this problem in 5.10.3 right now.

@asbjornu
Copy link
Member Author

The error you're seeing is not related to this, @mcascone, as GitVersion is not throwing the error message you're seeing. It is libgit2 trying to create the branch develop when Develop already exists, which is a filesystem case sensitivity issue that can only be resolved by you using all-lowercase branch names consistently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants