Skip to content

Fix: Editconfig#3023

Merged
shargon merged 23 commits intoneo-project:masterfrom
Jim8y:editconfig
Jan 2, 2024
Merged

Fix: Editconfig#3023
shargon merged 23 commits intoneo-project:masterfrom
Jim8y:editconfig

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
Jim8y
Jim8y commented Dec 13, 2023
View reviewed changes
Jim8y
Jim8y commented Dec 13, 2023
View reviewed changes
Jim8y
Jim8y commented Dec 13, 2023
View reviewed changes
@cschuchardt88
Copy link
Member

cschuchardt88 commented Dec 14, 2023

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

@cschuchardt88 cschuchardt88 Dec 14, 2023

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

@shargon shargon Dec 15, 2023

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

@cschuchardt88 cschuchardt88 Dec 15, 2023

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

@shargon shargon Dec 25, 2023

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

@shargon shargon Jan 2, 2024

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

cschuchardt88 commented Dec 20, 2023

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

cschuchardt88 commented Dec 20, 2023

@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
shargon
shargon reviewed Dec 25, 2023
View reviewed changes
shargon
shargon reviewed Dec 26, 2023
View reviewed changes
@cschuchardt88
Copy link
Member

cschuchardt88 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.

@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

cschuchardt88 commented Dec 26, 2023

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
@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

@shargon shargon shargon approved these changes

+1 more reviewer

@cschuchardt88 cschuchardt88 cschuchardt88 left review comments

Reviewers whose approvals may not affect merge requirements

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