Skip to content

[Refactor] Appended Async to async methods names#3218

Closed
cschuchardt88 wants to merge 22 commits intoneo-project:masterfrom
cschuchardt88:refactor5
Closed

[Refactor] Appended Async to async methods names#3218
cschuchardt88 wants to merge 22 commits intoneo-project:masterfrom
cschuchardt88:refactor5

Conversation

@cschuchardt88
Copy link
Member

@cschuchardt88 cschuchardt88 commented May 6, 2024

Change Log

  • Appended Async to async methods names
  • Updated .editorconfig with new rules, nothing in code changed besides above.

Needs #3214 to work

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Copy link
Member

@shargon shargon left a comment

Choose a reason for hiding this comment

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

We use reflection for smart contract methods names, this can't be done automatically

@cschuchardt88
Copy link
Member Author

cschuchardt88 commented May 7, 2024

Yes i know, I added see below to fix that issue. Plus all tests checkout... because before I had problem with that.

#pragma warning disable VSTHRD200 // Use "Async" suffix for async methods

@cschuchardt88 cschuchardt88 requested a review from shargon May 7, 2024 11:20
@cschuchardt88
Copy link
Member Author

Why is this taking too long?

Jim8y
Jim8y previously approved these changes May 17, 2024
@Jim8y Jim8y requested a review from a team May 27, 2024 12:08
@vncoelho
Copy link
Member

vncoelho commented May 27, 2024

We use reflection for smart contract methods names, this can't be done automatically

Was this comment from @shargon solved or replied?

@cschuchardt88
Copy link
Member Author

We use reflection for smart contract methods names, this can't be done automatically

Was this comment from @shargon solved or replied?

I say resolved, because it's been tested with the unit tests. Everything working fine.

}

internal async void Invoke(ApplicationEngine engine, byte version)
internal async Task InvokeAsync(ApplicationEngine engine, byte version)
Copy link
Member

Choose a reason for hiding this comment

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

I'm worry in line 392, when we check that it must be a ContractTask and now some method returns Task

Copy link
Member Author

@cschuchardt88 cschuchardt88 May 29, 2024

Choose a reason for hiding this comment

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

I already tested this. Plus if it was void then it needs to be task. I also tried ContractTask and didn't work.

@cschuchardt88 cschuchardt88 deleted the refactor5 branch June 24, 2024 05:39
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.

5 participants