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.

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

Neo-node Migration #2990

Merged
merged 4 commits into from
Dec 7, 2023
Merged

Conversation

lock9
Copy link
Contributor

@lock9 lock9 commented Nov 23, 2023

Migrating neo-node project into the Neo repository.

Observations:

  • The project names weren't following naming conventions.
    • neo-cli renamed to Neo.CLI
    • neo-gui renamed to Neo.GUI
    • Executable names remain the same
  • .editorconfig from neo-node was discarded in favor of the current one (more complete)
  • Docker file moved into the Neo.CLI project folder
    • It is used to build neo-cli
    • Passing framework parameter to the dotnet build command
    • The docker build was updated to match the new structure, but the image itself is broken
      • Tested on OSX. Maybe it's working on other platforms.
  • Neo-node doesn't have a Directory.Build. props file.
  • Neo-cli has a changelog file. This file wasn't migrated. Should it?
  • ConsoleService has its own version (currently 1.2.0)
  • The current neo workflow wasn't updated to distribute neo-cli
  • Neo-node has a codeql workflow that is not present on Neo and wasn't migrated.
  • There is a workflow that builds docker images but doesn't test it.
    • I didn't migrate this workflow since the Docker image doesn't seem to work, and running Docker may be resource-consuming.
  • I didn't test Neo GUI

Question:
Should I update the workflow to generate and test neo-cli?

@RichardBelsum
Copy link

Does anyone use the neo-gui project, Should we remove it?

@Jim8y
Copy link
Contributor

Jim8y commented Nov 23, 2023

Does anyone use the neo-gui project, Should we remove it?

Won't hurt to keep it. At least this pr only focus on merging existing code.

@lock9
Copy link
Contributor Author

lock9 commented Nov 23, 2023

Does anyone use the neo-gui project, Should we remove it?

I agree with @Liaojinghui. Let's avoid changing the structure during the migration.

@Jim8y
Copy link
Contributor

Jim8y commented Nov 24, 2023

@lock9 @shargon @roman-khimov @AnnaShaleva @superboyiii In case you feel it hard to verify the code, this script will help you to review that two folders (including all subdirs) of C# codes are exactly the same. If they are not, difference will be printed.

Directory Comparison Script

This script is designed to compare C# files between two directories (referred to as the original and merge directories), including their subdirectories. It ensures that not only the contents of the C# files are identical, but also that the directory structures and the number of files are the same.

Features

  • Recursive Comparison: The script recursively searches through all subdirectories in both the original and merge directories for C# files.
  • Content Verification: It compares each C# file in the original directory with its counterpart in the merge directory to ensure they are identical in content.
  • Structure Verification: The script checks that each C# file in the original directory has a corresponding file at the same relative path in the merge directory.
  • Difference Reporting: If any differences in content or structure are found, the script outputs the details of these differences.

Usage

  1. Provide Directories: Run the script with two arguments - the paths to the original and merge directories.
  2. Execute the Script: Save the script as compare_directories.sh, make it executable with chmod +x compare_directories.sh, and then run it as ./compare_directories.sh /path/to/original/dir /path/to/merge/dir.
#!/bin/bash

# Check if two arguments are passed
if [ "$#" -ne 2 ]; then
    echo "Usage: $0 <original_directory> <merge_directory>"
    exit 1
fi

original_dir=$1
merge_dir=$2

# Function to compare two directories
compare_directories() {
    local dir1=$1
    local dir2=$2

    # Get relative paths of all C# files in the first directory
    local files_dir1=($(cd "$dir1" && find . -type f -name "*.cs"))
    
    # Loop through each file in the first directory
    for file1 in "${files_dir1[@]}"; do
        local file2="${dir2}/${file1#./}"

        if [ ! -f "$file2" ]; then
            echo "File missing in $dir2: $file1"
            continue
        fi

        # Compare the contents of the two files
        if ! cmp -s "${dir1}/${file1}" "$file2"; then
            echo "Difference found in file: $file1"
            echo "----"
            diff "${dir1}/${file1}" "$file2"
            echo "----"
        fi
    done
}

# Compare original to merge
compare_directories "$original_dir" "$merge_dir"

# Compare merge to original (to catch any extra files in merge)
compare_directories "$merge_dir" "$original_dir"

@Jim8y
Copy link
Contributor

Jim8y commented Nov 24, 2023

We can also add it to the workflow to automatically check the code base.

name: Compare Codebases

on:
  pull_request:
    branches: [ master ]

jobs:
  compare:
    runs-on: ubuntu-latest
    steps:
    - name: Checkout Repository
      uses: actions/checkout@v2

    - name: Checkout Target Repository
      uses: actions/checkout@v2
      with:
        repository: 'https://github.com/neo-project/neo-node.git'
        ref: 'https://github.com/neo-project/neo-node/commit/da786195cfb6d70236bb37611c37d7016bbc3aae'
        path: 'neo-cli'

    - name: Compare Directories
      run: |
        chmod +x ./compare_directories.sh
        ./compare_directories.sh ./ ${{ github.workspace }}/Neo.CLI > difference.log || true

    - name: Upload differences if any
      uses: actions/upload-artifact@v2
      if: failure()
      with:
        name: differences
        path: difference.log

@roman-khimov
Copy link
Contributor

this script

I think I trust diff -puraN a bit more.

@roman-khimov roman-khimov added this to the v3.7.0 milestone Nov 24, 2023
@Jim8y
Copy link
Contributor

Jim8y commented Nov 24, 2023

this script

I think I trust diff -puraN a bit more.

As long as it can convince you to move monorepo faster.

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.

I think that we should wait the following PR (also for clean the issues in the repo):

@shargon
Copy link
Member

shargon commented Dec 5, 2023

@lock9 could you update the code?

@lock9
Copy link
Contributor Author

lock9 commented Dec 5, 2023

Hi @shargon,
I was confirming the PRs to be merged. I didn't see they were two. I'll update it today.

@shargon
Copy link
Member

shargon commented Dec 5, 2023

The issues are ready to be transferred to this repository, the description and tag have been updated.

lock9 and others added 2 commits December 5, 2023 20:10
Added blockchain show block/transactions/contracts commands neo-project#905 (csc…
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.

Dockerfile was removed? (found)

shargon
shargon previously approved these changes Dec 6, 2023
@shargon
Copy link
Member

shargon commented Dec 6, 2023

@lock9 we need to update the main README

@shargon shargon self-requested a review December 6, 2023 06:59
@lock9
Copy link
Contributor Author

lock9 commented Dec 7, 2023

@lock9 we need to update the main README

Ok.

coverage/coveralls — Coverage decreased (-51.5%) to 14.984%

What are we going to do about the code coverage? Add the UT?

@lock9
Copy link
Contributor Author

lock9 commented Dec 7, 2023

coverage/coveralls — Coverage remained the same at 66.505%

What happened? I just updated the readme...?

@shargon shargon merged commit 40d1086 into neo-project:master Dec 7, 2023
2 checks passed
Jim8y added a commit to Jim8y/neo that referenced this pull request Dec 31, 2023
* master: (30 commits)
  Set project as nullable (neo-project#3042)
  Fix: fix equal (neo-project#3028)
  Added README to packages (neo-project#3026)
  Nuget MyGet Fix (neo-project#3031)
  Add: print out the stack (neo-project#3033)
  fixed myget (neo-project#3029)
  Fixed MyGet Workflow (neo-project#3027)
  Package icons - hotfix (neo-project#3022)
  Nuget Package Icon & Symbols (neo-project#3020)
  Fix warning (neo-project#3021)
  Neo-node Migration (neo-project#2990)
  Remove unnecessary default seedlist (neo-project#2980)
  Fix Neo VM target frameworks (neo-project#2989)
  Update Neo.VM location in README.md (neo-project#2988)
  Migrating Neo VM (neo-project#2970)
  3.6.2 (neo-project#2962)
  fix ut (neo-project#2959)
  Validate serialization during Contract deploy and Update (neo-project#2948)
  code optimization (neo-project#2958)
  check null scriptcontainer (neo-project#2953)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants