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

Fix filter commit label #4926

Merged
merged 2 commits into from May 9, 2018

Conversation

pmiossec
Copy link
Member

@pmiossec pmiossec commented May 7, 2018

Changes proposed in this pull request:

  • Fix filter commit label which was displayed like "Commit..." instead of "Commit" because the transation was the one of the "commands" menu due to the same control name (a real problem in french translation for example).
  • Use of a much detailed label to better tell what the filter do (I, myself, never know that the filter was also used to search in sha1)

Screenshots before and after (if PR changes UI):

  • before:

image

  • after:

image

@codecov
Copy link

codecov bot commented May 7, 2018

Codecov Report

Merging #4926 into master will increase coverage by <.01%.
The diff coverage is 60%.

@@            Coverage Diff             @@
##           master    #4926      +/-   ##
==========================================
+ Coverage   34.52%   34.52%   +<.01%     
==========================================
  Files         542      542              
  Lines       43154    43154              
  Branches     5967     5967              
==========================================
+ Hits        14897    14899       +2     
+ Misses      27522    27520       -2     
  Partials      735      735

Copy link
Member

@gerhardol gerhardol left a comment

Choose a reason for hiding this comment

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

Suggest change to SHA in text, otherwise OK

_commitFilterToolStripMenuItem.Checked = true;
_commitFilterToolStripMenuItem.CheckOnClick = true;
_commitFilterToolStripMenuItem.Name = "commitToolStripMenuItem1";
_commitFilterToolStripMenuItem.Text = "Commit message and Sha1";
Copy link
Member

Choose a reason for hiding this comment

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

SHA-1 (and SHA) is used in similar commands

Copy link
Member

Choose a reason for hiding this comment

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

💡 I would actually use 'hash' instead of 'SHA'

Copy link
Member

Choose a reason for hiding this comment

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

I've heard some people use "commish" for "commit hash".
Either way we need to be consistent, and if we currently use SHA and SHA-1 then we need to continue with this.
We can reword it to "hash" or "commit hash" (or even "commish") separately before v3 is released

Copy link
Member Author

Choose a reason for hiding this comment

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

They surely use "commitish" ;-) but that is larger than just "commit hash" and refer to everything that end up pointing to a commit.

So, I think it is not understandable by most and also not exactly what we filter here (except if I'm wrong).

I like "hash" (or "SHA") which are more future proof :-)

I will update to "SHA" if that's we use at the moment.

Copy link
Member Author

Choose a reason for hiding this comment

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

finally, I have choosen "hash" because that's what is used in the commit details tab and in the "copy to clipboard" menu.

@RussKie RussKie added this to the 3.00 milestone May 8, 2018
@RussKie RussKie merged commit 7adb72b into gitextensions:master May 9, 2018
@pmiossec pmiossec deleted the fix_filter_commit_label branch August 2, 2018 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants