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.

Sign up for GitHub

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

Jump to bottom

Update editorconfig & fix warnings and errors appropriately #3417

Merged
merged 18 commits into from
Apr 16, 2024
Merged

Update editorconfig & fix warnings and errors appropriately #3417

merged 18 commits into from
Apr 16, 2024

Conversation

vexx32
Copy link
Member

@vexx32 vexx32 commented Apr 9, 2024

Description Of Changes

  • Update editorconfig per team discussions
  • Fix any warning or error-level complaints that resulted from this
  • Run dotnet format on everything to make sure the whitespace, indentation, etc., are all set

Motivation and Context

Keep code looking good and consistent.

Testing

  • Run CI tests. There should be no functional changes here.
  • Open project files in VS. There will be some Suggestion-level things remaining in many files, but no warnings or errors from the code style rules added/changed.

Operating Systems Testing

N/A

Change Types Made

  • Bug fix (non-breaking change).
  • Feature / Enhancement (non-breaking change).
  • Breaking change (fix or feature that could cause existing functionality to change).
  • Documentation changes.
  • PowerShell code changes.

Change Checklist

  • Requires a change to the documentation.
  • Documentation has been updated.
  • Tests to cover my changes, have been added.
  • All new and existing tests passed?
  • PowerShell code changes: PowerShell v2 compatibility checked?

Related Issue

Fixes #

@vexx32 vexx32 requested review from gep13 and AdmiringWorm April 9, 2024 18:40
@vexx32
Copy link
Member Author

vexx32 commented Apr 9, 2024

Kinda iffy on whether this needed an issue opened or not, if y'all want me to open an issue and re-tag stuff against it, let me know?

@gep13 gep13 force-pushed the editorconfig-update branch from 9bcdd2e to 57ef208 Compare April 16, 2024 12:51
gep13
gep13 previously approved these changes Apr 16, 2024
View reviewed changes
Copy link
Member

@gep13 gep13 left a comment

Choose a reason for hiding this comment

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

LGTM!

@gep13
Copy link
Member

gep13 commented Apr 16, 2024

@vexx32 said...
Kinda iffy on whether this needed an issue opened or not, if y'all want me to open an issue and re-tag stuff against it, let me know?

I think I am actually okay with this just going through as (maint) commits. They are logically broken up per rule that is being worked on, which made it super easy to review this!

Thank you very much for getting this done!

@gep13
Copy link
Member

gep13 commented Apr 16, 2024

@vexx32 looks like at least one unit test is now failing as a result of the changes that you have made. Are you in a position to take a look at this, and get it fixed up?

vexx32 added 18 commits April 16, 2024 09:36
@vexx32
(maint) Update editorconfig per team preferences
39aa061 
Following on from the team's meetings about code style preferences,
updating the editorconfig to match the desired preferences.
@vexx32
(maint) Disable style rules for option.cs
c8f0702 
This file we tend not to touch, so evaluating every style rule here is
a bit pointless, especially as we want to try replacing this when we get
an opportunity.
@vexx32
(maint) Disable warnings on deprecated methods
83843b7 
These overloads will be removed anyway, we don't need the warning noise
from these.
@vexx32
(maint) Fix IDE1006 error
44af99d 
Naming conventions
@vexx32
(maint) Fix IDE0011 errors
976d855 
Use braces for if statements
@vexx32
(maint) Fix IDE0065 warnings
6735897 
Place using statements outside namespace block
@vexx32
(maint) Fix IDE0032 warnings
8c42230 
Use auto-property
@vexx32
(maint) Fix IDE0049 warnings
cbde060 
Simplify type names -> primitives keywords
@vexx32
(maint) fix IDE0007 warnings
da3642b 
Use var instead of explicit type
@vexx32
(maint) Fix IDE0028 warnings
61aa6eb 
Simplify colleciton initialisation
@vexx32
(maint) fix IDE0041 warnings
365c036 
Simplify null check
@vexx32
(maint) Fix IDE0040 errors
caf9002 
Require accessibility modifiers
@vexx32
(maint) Fix IDE0017 warnings
1181602 
Simplify object initialisation
@vexx32
(maint) Fix IDE0060 errors
ce11404 
unused method parameters
@vexx32
(maint) Fix IDE0003 errors
5edbd85 
Avoid using `this`
@vexx32
(maint) Fix IDE0027 warnings
54e44ff 
Use block body for accessors
@vexx32
(maint) Fix IDE0025 warnings
8b0e4b9 
Use block body for properties
@vexx32
(maint) dotnet format
8d2425c 
Just to tidy up the whitespace and everything, make everything neat and
tidy and consistent.
@vexx32 vexx32 force-pushed the editorconfig-update branch from 57ef208 to 8d2425c Compare April 16, 2024 13:43
@gep13
Copy link
Member

gep13 commented Apr 16, 2024

@vexx32 I am going to move forward with merging this PR, as the failing Docker Build is not related to the changes that you have made.

@gep13 gep13 merged commit ff13a5d into chocolatey:develop Apr 16, 2024
4 of 5 checks passed
@gep13
Copy link
Member

gep13 commented Apr 16, 2024

@vexx32 thanks again for working through all these rules and fixing up all the required changes!

@vexx32 vexx32 deleted the editorconfig-update branch April 16, 2024 17:59
@gep13 gep13 mentioned this pull request Apr 17, 2024
Closed
@gep13 gep13 linked an issue Apr 17, 2024 that may be closed by this pull request
Define the naming conventions and formatting #13
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gep13 gep13 gep13 left review comments

@AdmiringWorm AdmiringWorm Awaiting requested review from AdmiringWorm

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Define the naming conventions and formatting
2 participants
@vexx32 @gep13