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

Change some default settings values #5037

Merged

Conversation

pmiossec
Copy link
Member

@pmiossec pmiossec commented Jun 5, 2018

A beginning following the discussion in #5030

Changes proposed in this pull request:

  • In revision grid, "Show SHA-1" and "Show indicator for multiline message" are enabled by default
  • In commit form, "Mark ill formed lines" is enabled by default

What did I do to test the code and ensure quality:

  • I removed the folder %APPDATA%\GitExtensions to restart with the default settings
  • Launch the application
  • Verify that the 3 settings are enabled by default

Has been tested on (remove any that don't apply):

  • GIT 2.17
  • Windows 10

In revision grid, "Show SHA-1" and "Show indicator for multiline message" are enabled by default
In commit form, "Mark ill formed lines" is enabled by default
@pmiossec pmiossec changed the title Change some default settings Change some default settings values Jun 5, 2018
@codecov
Copy link

codecov bot commented Jun 5, 2018

Codecov Report

Merging #5037 into master will increase coverage by 0.11%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #5037      +/-   ##
==========================================
+ Coverage   34.65%   34.76%   +0.11%     
==========================================
  Files         563      563              
  Lines       44528    44528              
  Branches     6148     6148              
==========================================
+ Hits        15431    15482      +51     
+ Misses      28342    28273      -69     
- Partials      755      773      +18

@@ -876,7 +876,7 @@ public static bool ShowGitNotes

public static bool ShowIndicatorForMultilineMessage
{
get => GetBool("showindicatorformultilinemessage", false);
get => GetBool("showindicatorformultilinemessage", true);
Copy link
Member

@gerhardol gerhardol Jun 6, 2018

Choose a reason for hiding this comment

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

This setting has performance implications, slower loading. OK for me, I use the setting.

Copy link
Member

Choose a reason for hiding this comment

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

Oh thanks, I wasn't particularly aware of this.
Perhaps we want to keep it disabled - our work laptops are crippled with McAfee and I wouldn't want to make engineers even less efficient than they are already...

Copy link
Member

Choose a reason for hiding this comment

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

Reverting my comment: drew fixed this in #4988, this is no longer limiting performance.

Copy link
Member

@drewnoakes drewnoakes left a comment

Choose a reason for hiding this comment

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

Looks good. I'm not completely sure about showing IDs by default, but happy enough either way.

@gerhardol gerhardol added the ready label Jun 7, 2018
@drewnoakes drewnoakes merged commit d1e8ca9 into gitextensions:master Jun 13, 2018
@RussKie RussKie removed the ready label Jul 3, 2018
@pmiossec pmiossec deleted the change_some_default_settings branch August 2, 2018 07:50
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