-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Fixed up main.yml
#3292
Fixed up main.yml
#3292
Conversation
This reverts commit 5ba38b1.
Co-authored-by: Shargon <shargon@gmail.com>
I will review soon. Do not merge. |
- name: Check Format (*.cs) | ||
run: dotnet format --verify-no-changes --verbosity diagnostic | ||
|
||
- name: Build (Neo.CLI) | ||
run: | | ||
dotnet build ./src/Neo.CLI \ | ||
--output ./out/Neo.CLI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a side comment: it might be a good idea to split this job into Format
and Build
, because Format
job itself does not block the Test
job. And then Test
job may be dependent only on Build
job. It's just the way how it works for NeoGo actions, and sometimes it's useful because your code may have a lack of formatting and you'll get your test results anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a required step/job. That is why its linked to tests. Or else @shargon would have to make the build
and format
job required for PRs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine, or it can be done in a different PR, if the format is wrong we get the error earlier
Coverage decreased (-3.0%) to 71.386% |
@@ -5,6 +5,8 @@ | |||
<TargetFrameworks>netstandard2.1;net8.0</TargetFrameworks> | |||
<ImplicitUsings>enable</ImplicitUsings> | |||
<Nullable>enable</Nullable> | |||
<PackageId>Neo.Cryptography.BLS12_381</PackageId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed, what is this packageID need?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this package created here or elsewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I searched quickly here and found that packageId was more related to dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its for nuget
. The nuget
filename and package name of the package.
@@ -7,6 +7,9 @@ | |||
<IsPackable>false</IsPackable> | |||
<IsTestProject>true</IsTestProject> | |||
<WarningLevel>0</WarningLevel> | |||
<CollectCoverage>true</CollectCoverage> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this good? Because this is generic to any OS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this enables for all tests. I don't know what you mean by generic to any OS. Does it work on all... yes.
switched back to |
* 'fix-3300' of github.com:Jim8y/neo: Fixed up `main.yml` (neo-project#3292) [Neo Core Style] Rename parameters, update comments, fix code style (neo-project#3294)
Why did you merge this PR? The coverage had problems and decreased. |
* master: Fix error when vc_redist is missing (neo-project#3293) Fixed up `main.yml` (neo-project#3292) [Neo Core Style] Rename parameters, update comments, fix code style (neo-project#3294)
Change Log
BaseOutputPath
for @vncoelhoplugins
Coveralls
toDirectory.Build.props
Coveralls
lcov
formatNow all binary outputs will be at
$(SolutionDir)/bin/$(PackageId)
Example
.\neo\bin\Neo\Debug\net8.0
.\neo\bin\Neo.Plugins.ApplicationLogs\Debug\net8.0\
Type of change
How Has This Been Tested?
Test Configuration:
Checklist: