[Part-1] `neo-module/master` #3232

cschuchardt88 · 2024-05-14T03:09:23Z

Change Log

Moved neo-modules/src/ to directory src
Moved neo-modules/tests/ to directory tests
Configured .github workflows for tests.
Run dotnet format
Added neo-modules projects to neo.sln file.

Closes #3230

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

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

@shargon @Jim8y @vncoelho @superboyiii

…ows`

shargon · 2024-05-14T08:07:40Z

what is part 2?

.editorconfig

.github/workflows/main.yml

Not related to the PR

.github/workflows/main.yml

vncoelho

I see that you moved modules inside the root folder.
Why not src?

cschuchardt88 · 2024-05-14T14:54:54Z

I see that you moved modules inside the root folder. Why not src?

Because I figured it would make more sense to keep them separate.

Its unrelated to core development. (their plugins)
Easier to find and maintain.

what is part 2?

Any of the commits above you don't want, would be part 2 or unrelated un-needed changes/fixes. Part 1 is just to make a working solution.

vncoelho · 2024-05-14T17:51:45Z

@cschuchardt88,
I think it should be in the SRC in this first moment, like the others.

Furthermore, avoid any other change in files as we had discussed previously.
Thus, use any change in editorconfig in an additional PR.

vncoelho

I think that using the name plugins instead of modules is also great for the folder

cschuchardt88 · 2024-05-14T18:00:24Z

At least with plugins name we limit our self to only allowing plugins in this folder. But that can be done In the sln file to show. It can be big directory with +13 items now being added. Becoming to hard to manage with unrelated items in src directory.

.github/workflows/main.yml

vncoelho · 2024-05-20T10:15:15Z

I think it will be correct with the last commit, just waiting for the tests

vncoelho · 2024-05-20T10:18:43Z

In principle, checking the logs, it looks like everything was fine.
However, the coverage summary is not directly appearing on the PR.

cschuchardt88 · 2024-05-20T10:31:40Z

In principle, checking the logs, it looks like everything was fine. However, the coverage summary is not directly appearing on the PR.

I believe if it doesn't than coverage didn't change.

vncoelho · 2024-05-20T10:41:50Z

For Plugins the coverage is correct.
"coverage/coveralls (push) - Coverage remained the same at 48.583%" -> This is from the last commit on neo-modules

vncoelho · 2024-05-20T10:43:02Z

The other also increased a little. I think it is all correct.
We just have some display problems here in the PR. We can fix later if they persist.

vncoelho

I am adjusting my tooling and will test in some minutes.

vncoelho

#36 71.70 /usr/share/dotnet/sdk/8.0.203/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.ConflictResolution.targets(112,5): error NETSDK1152: Found multiple publish output files with the same relative path: /opt/neoLib/src/Plugins/RpcServer/config.json, /opt/neoLib/src/Plugins/StateService/config.json. [/opt/neoLib/src/Plugins/StateService/StateService.csproj::TargetFramework=net8.0]

vncoelho · 2024-05-20T13:32:51Z

For me it is ready to merge.

Code is the same as neo-modules apart from necessary changes.
Compilation, tests and publish are all good in my local environment.

However, code is not running due to recent PRs that introduces some problems on wallet.
I think we can merge and then fix in order to move forward.

Jim8y · 2024-05-20T13:36:08Z

If code is good, we can go with this pr already. Not necessarily to complete everything in one pr. But maybe we need a development branch? @shargon

cschuchardt88 · 2024-05-20T13:36:46Z

I have #3244 for changes

vncoelho · 2024-05-20T13:37:03Z

If code is good, we can go with this pr already. Not necessarily to complete everything in one pr. But maybe we need a development branch? @shargon

If we go to a development branch please revert the last 3 commits as well and migrate them to such branch.

shargon · 2024-05-20T13:42:59Z

Give me a couple of hours for the comparison

vncoelho · 2024-05-20T14:10:46Z

@Jim8y , I think we can try with the master branch a little bit more now that we have "all necessary components" here.
I think that by pushing a UT that tests node's ability to start a consensus and do basic operations will be very good. Maybe we can keep working on main branch in such case.

But I am not against using a dev branch.

roman-khimov

Code-wise (at 68e4146) I've noticed some s/this// changes and two lines moved in src/Plugins/RpcClient/Models/RpcInvokeResult.cs compared to master of neo-modules. Minor, but can be fixed to have cleaner initial state here and stylistic changes in separate PRs.

Can't comment on csproj/workflows changes, likely OK.

vncoelho

Perhaps the aforementioned problem I mentioned on discord was not introduced on 08f2dfc56762bb43be33e873bc3659bad368d4d6, but here.

In this case, please do not merge until I finish my tests because the error here may take time to solve otherwise.

vncoelho

The error was, in fact, a typo on the new configuration file name on Part-2 PR.
Thus, it is approved now with tests running as expected.

shargon

The changes with neo-modules are understandable, to reduce warnings only "this" and a few things were removed.

…gins * 'latest-plugins' of github.com:Jim8y/neo: (21 commits) fix: custom plugins won't shown by command `plugins` (neo-project#3269) COVERALL: Improve maintenance and readbility of some variables (neo-project#3248) Update nuget (neo-project#3262) [**Part-2**] Neo module/master fixes (neo-project#3244) Fix `dotnet pack` error (neo-project#3266) Fix and Update devcontainer.json to use Dockerfile (neo-project#3259) Add optimization to template (neo-project#3247) Optimize plugin's models (neo-project#3246) fix CancelTransaction !signers.Any() (neo-project#3263) COVERALL: fix broken by changing report from lcov to cobertura (neo-project#3252) fix TraverseIterator count (neo-project#3261) Native: include DeprecatedIn hardfork into usedHardforks (neo-project#3245) [**Part-1**] `neo-module/master` (neo-project#3232) Make `ApplicationEngine.LoadContext` protection level `public` (neo-project#3243) improve parse method in neo-cli (neo-project#3204) Fix neo-project#3239 (neo-project#3242) Neo.CLI: enable hardforks for NeoFS mainnet (neo-project#3240) v3.7.4 (neo-project#3237) fix hardfork issues (neo-project#3234) Update src/Neo.CLI/CLI/MainService.Plugins.cs ... # Conflicts: # src/Neo.CLI/CLI/MainService.Plugins.cs

cschuchardt88 added 11 commits May 13, 2024 22:04

Moved neo-modules to neo

40614dd

Configured module projects

b1bad7c

Configured Test Projects

69c8ded

Dotnet format

5ffd0b1

Update github workflow for tests

143db61

removed dupes in .editorconfig

fc17bed

remove dupe file

c66a438

Fixed waarning

d185b7a

fixed macOS workflow for leveldb

0453f0a

Added EXCLUDED_TESTS for Neo.Plugins.Storage on macOS and `Wind…

4e9f0d8

…ows`

Fixed workflow tests for windows

483c831

cschuchardt88 marked this pull request as draft May 14, 2024 03:45

cschuchardt88 added 3 commits May 13, 2024 23:46

Fixed char ecsape for tests on windows

3902acf

Fixed tests

e354cbd

Fixed tests

71105dd

cschuchardt88 marked this pull request as ready for review May 14, 2024 04:12

shargon reviewed May 14, 2024

View reviewed changes

Apply suggestions from code review

e13b8a2

Not related to the PR

shargon reviewed May 14, 2024

View reviewed changes

.github/workflows/main.yml Outdated Show resolved Hide resolved

.github/workflows/main.yml Outdated Show resolved Hide resolved

.github/workflows/main.yml Outdated Show resolved Hide resolved

.github/workflows/main.yml Outdated Show resolved Hide resolved

Apply suggestions from code review

79fe63f

shargon reviewed May 14, 2024

View reviewed changes

.github/workflows/main.yml Show resolved Hide resolved

vncoelho reviewed May 14, 2024

View reviewed changes

cschuchardt88 added 3 commits May 14, 2024 15:08

Update .editorconfig

f80b145

Update .editorconfig

3289c9c

Changed for @vncoelho request

9e3017b

cschuchardt88 commented May 20, 2024

View reviewed changes

.github/workflows/main.yml Outdated Show resolved Hide resolved

Update .github/workflows/main.yml

81ad31a

vncoelho reviewed May 20, 2024

View reviewed changes

.github/workflows/main.yml Show resolved Hide resolved

cschuchardt88 commented May 20, 2024

View reviewed changes

.github/workflows/main.yml Outdated Show resolved Hide resolved

Update .github/workflows/main.yml

68e4146

vncoelho reviewed May 20, 2024

View reviewed changes

cschuchardt88 changed the title ~~[**MOVE**] neo-module/master~~ [**Part-1**] neo-module/master May 20, 2024

vncoelho approved these changes May 20, 2024

View reviewed changes

Jim8y approved these changes May 20, 2024

View reviewed changes

roman-khimov reviewed May 20, 2024

View reviewed changes

Merge branch 'master' into neo-module/master

232092f

vncoelho requested changes May 20, 2024

View reviewed changes

vncoelho approved these changes May 20, 2024

View reviewed changes

shargon approved these changes May 21, 2024

View reviewed changes

shargon merged commit 0f04c7f into neo-project:master May 21, 2024
4 of 5 checks passed

shargon deleted the neo-module/master branch May 21, 2024 08:01

AnnaShaleva mentioned this pull request May 21, 2024

Take care of C# node dockerfiles nspcc-dev/neo-bench#175

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Part-1] `neo-module/master` #3232

[Part-1] `neo-module/master` #3232

cschuchardt88 commented May 14, 2024 •

edited

Loading

shargon commented May 14, 2024

vncoelho left a comment

cschuchardt88 commented May 14, 2024

vncoelho commented May 14, 2024

vncoelho left a comment

cschuchardt88 commented May 14, 2024 •

edited

Loading

vncoelho commented May 20, 2024 •

edited

Loading

vncoelho commented May 20, 2024

cschuchardt88 commented May 20, 2024 •

edited

Loading

vncoelho commented May 20, 2024

vncoelho commented May 20, 2024

vncoelho left a comment

vncoelho left a comment

vncoelho commented May 20, 2024

Jim8y commented May 20, 2024

cschuchardt88 commented May 20, 2024

vncoelho commented May 20, 2024 •

edited

Loading

shargon commented May 20, 2024

vncoelho commented May 20, 2024

roman-khimov left a comment

vncoelho left a comment

vncoelho left a comment

shargon left a comment

[**Part-1**] neo-module/master #3232

[**Part-1**] neo-module/master #3232

Conversation

cschuchardt88 commented May 14, 2024 • edited Loading

Change Log

Type of change

How Has This Been Tested?

Checklist:

shargon commented May 14, 2024

vncoelho left a comment

Choose a reason for hiding this comment

cschuchardt88 commented May 14, 2024

vncoelho commented May 14, 2024

vncoelho left a comment

Choose a reason for hiding this comment

cschuchardt88 commented May 14, 2024 • edited Loading

vncoelho commented May 20, 2024 • edited Loading

vncoelho commented May 20, 2024

cschuchardt88 commented May 20, 2024 • edited Loading

vncoelho commented May 20, 2024

vncoelho commented May 20, 2024

vncoelho left a comment

Choose a reason for hiding this comment

vncoelho left a comment

Choose a reason for hiding this comment

vncoelho commented May 20, 2024

Jim8y commented May 20, 2024

cschuchardt88 commented May 20, 2024

vncoelho commented May 20, 2024 • edited Loading

shargon commented May 20, 2024

vncoelho commented May 20, 2024

roman-khimov left a comment

Choose a reason for hiding this comment

vncoelho left a comment

Choose a reason for hiding this comment

vncoelho left a comment

Choose a reason for hiding this comment

shargon left a comment

Choose a reason for hiding this comment

[Part-1] `neo-module/master` #3232

[Part-1] `neo-module/master` #3232

cschuchardt88 commented May 14, 2024 •

edited

Loading

cschuchardt88 commented May 14, 2024 •

edited

Loading

vncoelho commented May 20, 2024 •

edited

Loading

cschuchardt88 commented May 20, 2024 •

edited

Loading

vncoelho commented May 20, 2024 •

edited

Loading