Skip to content

Conversation

benrr101
Copy link
Contributor

@benrr101 benrr101 commented Aug 9, 2024

Description: One thing that lights up my IDE a lot is inconsistent ordering of modifiers for methods, properties, etc. This PR does a quick sweep of the codebase (automated) and reorders the modifiers to be consistent with C# standards.

AI Analysis:
image

Testing: Well ... if it builds and passes the CI tests, I think it's good to go.

@benrr101 benrr101 added the Code Health 💊 Issues/PRs that are targeted to source code quality improvements. label Aug 9, 2024
@saurabh500
Copy link
Contributor

saurabh500 commented Aug 9, 2024

I like your changes, but this is going to cause me a lot of pain when we have to bring in the JSON changes from the feature branch, after the protocol changes are published

@roji
Copy link
Member

roji commented Aug 9, 2024

For this kind of code style change, I'd highly recommend encoding the applied style in .editorconfig, so that discrepancies don't creep back in in the future.

@saurabh500
Copy link
Contributor

Please hold off on the merge of this PR till we drive #2714 and JSON work to conclusion.

@JRahnama
Copy link
Contributor

JRahnama commented Aug 9, 2024

@saurabh500 I think, we can also have RoslynAnalyzer to enforce some coding standards to prevent this in future as this cannot be done in editorconfig file.

Copy link

codecov bot commented Aug 9, 2024

Codecov Report

Attention: Patch coverage is 90.32258% with 3 lines in your changes missing coverage. Please review.

Project coverage is 71.73%. Comparing base (7216e84) to head (c24dc2b).
Report is 118 commits behind head on main.

Files with missing lines Patch % Lines
...netfx/src/Microsoft/Data/SqlTypes/SqlFileStream.cs 0.00% 1 Missing ⚠️
...SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs 0.00% 1 Missing ⚠️
...rc/Microsoft/Data/SqlClient/SqlInfoMessageEvent.cs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2765      +/-   ##
==========================================
+ Coverage   71.70%   71.73%   +0.02%     
==========================================
  Files         308      308              
  Lines       62314    62314              
==========================================
+ Hits        44682    44698      +16     
+ Misses      17632    17616      -16     
Flag Coverage Δ
addons 92.90% <100.00%> (ø)
netcore 75.93% <71.42%> (-0.03%) ⬇️
netfx 69.74% <89.65%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@benrr101
Copy link
Contributor Author

benrr101 commented Aug 9, 2024

@roji Ok, I've added it to the editorconfig at the root of the repo
@JRahnama according to a stack overflow, this can be checked as part of editorconfig. 🤷‍♂️
@saurabh500 I don't think it will cause too many headaches for y'all, but ok.

@DavoudEshtehari DavoudEshtehari added this to the 6.0-preview2 milestone Sep 18, 2024
@cheenamalhotra
Copy link
Member

@benrr101 can you please resolve these conflicts?

@benrr101
Copy link
Contributor Author

benrr101 commented Nov 6, 2024

Closing as too many things have changed since last working on this. BUT I HAVEN'T GIVEN UP 😤

@benrr101 benrr101 closed this Nov 6, 2024
@cheenamalhotra cheenamalhotra removed this from the 6.0-preview3 milestone Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Health 💊 Issues/PRs that are targeted to source code quality improvements.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants