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

Fix: Editconfig #3023

Merged
merged 23 commits into from
Jan 2, 2024
Merged

Fix: Editconfig #3023

merged 23 commits into from
Jan 2, 2024

Conversation

Jim8y
Copy link
Contributor

@Jim8y Jim8y commented Dec 13, 2023

This pr adopts https://github.com/dotnet/roslyn editconfig to restrain the coding style.

cschuchardt88
cschuchardt88 reviewed Dec 13, 2023
View reviewed changes
.editorconfig Outdated Show resolved Hide resolved
.editorconfig Show resolved Hide resolved
benchmarks/Neo.Benchmarks/Benchmarks.cs Outdated Show resolved Hide resolved
Jim8y
Jim8y commented Dec 13, 2023
View reviewed changes
.editorconfig Outdated Show resolved Hide resolved
Jim8y
Jim8y commented Dec 13, 2023
View reviewed changes
.editorconfig Show resolved Hide resolved
Jim8y
Jim8y commented Dec 13, 2023
View reviewed changes
.editorconfig Show resolved Hide resolved
@cschuchardt88
Copy link
Member

Maybe to want to add #3019 to this PR, going to have big conflict on one or the other.

cschuchardt88
cschuchardt88 reviewed Dec 14, 2023
View reviewed changes
.editorconfig Outdated
[*.{cs,vb}]

# Sort using and Import directives with System.* appearing first
dotnet_sort_system_directives_first = true
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
dotnet_sort_system_directives_first = true
dotnet_sort_system_directives_first = false

Copy link
Member

Choose a reason for hiding this comment

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

Default value is true (https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/dotnet-formatting-options)

Copy link
Member

Choose a reason for hiding this comment

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

ya but this is annoying, should be alphabetical.

Copy link
Member

Choose a reason for hiding this comment

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

Why? I think that if this is the case by default, there is a reason.

Copy link
Member

Choose a reason for hiding this comment

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

ya but this is annoying, should be alphabetical.

The same argument as my var :)

@cschuchardt88
Copy link
Member

want me to update this for @Jim8y ?

@Jim8y
Copy link
Contributor Author

Jim8y commented Dec 20, 2023

want me to update this for @Jim8y ?

Please

@cschuchardt88
Copy link
Member

@Jim8y I updated that test for HashCodeTest as well. It's all complete now.

@Jim8y
Copy link
Contributor Author

Jim8y commented Dec 20, 2023

@shargon may you please check and review this.
@superboyiii may you also check if the code rules here are reasonable.

shargon
shargon reviewed Dec 25, 2023
View reviewed changes
.editorconfig Outdated Show resolved Hide resolved
.editorconfig Outdated Show resolved Hide resolved
shargon
shargon reviewed Dec 25, 2023
View reviewed changes
src/Neo/Wallets/Wallet.cs Outdated Show resolved Hide resolved
shargon
shargon reviewed Dec 26, 2023
View reviewed changes
.editorconfig Show resolved Hide resolved
@cschuchardt88
Copy link
Member

@shargon why did you remove globals.json? We need that for the workflow to work correctly on github, atm it compiling using dotnet 8.0 sdk. neo-devpack-dotnet had a problem with 8.0. I know this repo is capable with dotnet 8.0 at the moment. But we shouldn't be using it, if we don't update to 8.0. dotnet 8.0 has a lot of new features that are not compatible with dotnet 7.0.

@shargon
Copy link
Member

shargon commented Dec 26, 2023

@shargon why did you remove globals.json? We need that for the workflow to work correctly on github, atm it compiling using dotnet 8.0 sdk. neo-devpack-dotnet had a problem with 8.0. I know this repo is capable with dotnet 8.0 at the moment. But we shouldn't be using it, if we don't update to 8.0. dotnet 8.0 has a lot of new features that are not compatible with dotnet 7.0.

Please use a different PR for this change

@cschuchardt88
Copy link
Member

I have it in #3031 also, but my thoughts were, this PR would come 1st.

@shargon shargon merged commit 13f506c into neo-project:master Jan 2, 2024
1 of 2 checks passed
@Jim8y Jim8y deleted the editconfig branch January 2, 2024 16:13
Jim8y added a commit to Jim8y/neo that referenced this pull request Jan 10, 2024
@Jim8y
Merge branch 'master' into commandline
07b246a 
* master:
  Fixed asp.net core project (neo-project#3067)
  Updated BLS12_381 (neo-project#3074)
  avoid nonsense exception messages. (neo-project#3063)
  Removed `MyGet` (neo-project#3071)
  Updated unit-test (neo-project#3073)
  add hash verification for OnImport (neo-project#3070)
  Make public ReadUserInput (neo-project#3068)
  Removed asp.net core (neo-project#3065)
  Enforce Line Endings in `.editorconfig` (neo-project#3060)
  Remove some warnings (neo-project#3057)
  Fixed workflow  timeout-minutes (neo-project#3048)
  Fix: specify the error message (neo-project#3030)
  Removes `WebSocket`s from the network layer (neo-project#3039)
  set timeout for tests (neo-project#3046)
  Fix: Editconfig (neo-project#3023)
  Set project as nullable (neo-project#3042)
  Fix: fix equal (neo-project#3028)

# Conflicts:
#	src/Neo.CLI/CLI/MainService.cs
#	src/Neo.CLI/Settings.cs
#	src/Neo/ProtocolSettings.cs
@cschuchardt88 cschuchardt88 mentioned this pull request May 8, 2024
Closed
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cschuchardt88 cschuchardt88 cschuchardt88 left review comments

@shargon shargon shargon approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@Jim8y @cschuchardt88 @shargon