-
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
Changes from 38 commits
8b64351
88f521e
d6e628b
6b784a2
b095480
f4972ce
ca4ae65
823f8a6
0bfe9e5
5ba38b1
1a1335d
6af1f81
0e8c10c
ac260b0
e51a3a1
d311bef
ac8d508
70ace83
aab490f
8ea70ae
fa7bbfa
5cfc2a3
57e1150
4116b85
2ce14f4
f395f73
a7f882c
15c57b8
d03a9a0
4389801
e803906
a1d6fe4
0526e8d
4730ab7
3bcd995
9e5468b
2ddcffa
f5c0133
75ad6e3
2f1d5b3
4bae6c5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 commentThe 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 commentThe 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 commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. Its for |
||
<BaseOutputPath>$(SolutionDir)/bin/$(PackageId)</BaseOutputPath> | ||
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 commentThe 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 commentThe 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. |
||
<CoverletOutput>TestResults/</CoverletOutput> | ||
<CoverletOutputFormat>lcov</CoverletOutputFormat> | ||
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
|
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
andBuild
, becauseFormat
job itself does not block theTest
job. And thenTest
job may be dependent only onBuild
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
andformat
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