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

Remove redundant qualifiers #2342

Merged
merged 2 commits into from Oct 13, 2017
Merged

Remove redundant qualifiers #2342

merged 2 commits into from Oct 13, 2017

Conversation

c0shea
Copy link
Contributor

@c0shea c0shea commented Oct 12, 2017

I inadvertently did a force push, which closed #2336 and I couldn't seem to get it back. This PR is a redo now that #2327 has been merged.

To continue on our previous discussion, I'll create a new PR for expression-bodied members once this one is merged to avoid more conflicts.

I personally love to use var for everything. I know some people hate it because they want to see the types explicitly spelled out, but to me that's too verbose. If I want to know the type, I can just hover over the variable ;) That said, I could go either way depending on your thoughts. I think it would make the most sense for complex expressions, e.g. LINQ and other nested generic types, but it would be fine to keep using string test = "123" instead of making that a var.

@codecov
Copy link

codecov bot commented Oct 13, 2017

Codecov Report

Merging #2342 into master will decrease coverage by <1%.
The diff coverage is 84%.

@@           Coverage Diff           @@
##           master   #2342    +/-   ##
=======================================
- Coverage      82%     82%   -<1%     
=======================================
  Files         318     318            
  Lines       22771   22770     -1     
  Branches     2782    2782            
=======================================
- Hits        18668   18660     -8     
- Misses       3401    3409     +8     
+ Partials      702     701     -1

@c0shea
Copy link
Contributor Author

c0shea commented Oct 13, 2017

@304NotModified Not sure why Travis is failing the build with The command "sudo apt-key adv --keyserver keyserver.ubuntu.com --recv-keys 3FA7E0328081BFF6A14DA29AA6A19B38D3D831EF" failed and exited with 2 during . after I resolved the latest conflicts. Thoughts?

@304NotModified
Copy link
Member

@304NotModified Not sure why Travis is failing the build with The command "sudo apt-key adv --keyserver keyserver.ubuntu.com --recv-keys 3FA7E0328081BFF6A14DA29AA6A19B38D3D831EF" failed and exited with 2 during . after I resolved the latest conflicts. Thoughts?

Travis is buggy. Will retry (a few times).

@304NotModified
Copy link
Member

worked after 1 retry :)

Double check, you used R# for this isn't?

@304NotModified 304NotModified added this to the 4.5 beta 7 milestone Oct 13, 2017
@c0shea
Copy link
Contributor Author

c0shea commented Oct 13, 2017

Yes, I used ReSharper for this. So nice to do it in s couple clicks for the solution.

@304NotModified 304NotModified merged commit 74e6e26 into NLog:master Oct 13, 2017
@304NotModified
Copy link
Member

thanks!!

@304NotModified 304NotModified requested review from 304NotModified and removed request for 304NotModified October 13, 2017 15:50
@snakefoot
Copy link
Contributor

Must admit I kind of liked the NLog-prefix for the NLog-namespace specifications. But the removal of explicit this is great.

@c0shea c0shea deleted the redundant-qualifiers branch October 14, 2017 17:00
@snakefoot snakefoot modified the milestones: 4.5 beta 7, 4.5 Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants