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

Migrate build system to net8 / fake v6 #361

Merged
merged 7 commits into from
Apr 19, 2024

Conversation

anpin
Copy link
Contributor

@anpin anpin commented Apr 17, 2024

This PR updates build scripts to FAKE v6 and updates test targets to net8.0 thus reducing the need for developer to have multiple outdated build tools to get started.

Fixes
#360
#362

Changes

  • Updated FAKE from v4 to v6 and removed old bootstrap scripts
  • NetCoreTestVersion updated to net8.0 (which required MySql.Data to be updated to latest version)
  • DocFx updated to latest version which uses dotnet-tools (allows working with docs on mac/linux)
  • DocFx serve is invoked as a part of build.fsx and serve-docs.[cmd|sh] are updated to use that
  • Fixed broken links in docs
  • Refactored docker container initialization to fix inconsistent test runs
  • Adds benchmarks to the build script

Checklist

For significant changes, please ensure that the following have been completed (delete if not relevant):

Latest dev Benchmarks

Include data from the relevant benchmark prior to this change here.

This PR's Benchmarks

Include data from after this change here.

@anpin
Copy link
Contributor Author

anpin commented Apr 17, 2024

currently runs locally on linux, windows not tested yet. Plus there are some errors in the tests themselves, probably due to mismatched dependencies.

@anpin anpin changed the title migrate build system to net8 / fake v6 [WIP] migrate build system to net8 / fake v6 Apr 17, 2024
@anpin anpin force-pushed the feature/net8 branch 2 times, most recently from 3957af5 to 7edda72 Compare April 17, 2024 22:24
@anpin anpin force-pushed the feature/net8 branch 4 times, most recently from dd312c4 to 94922c3 Compare April 18, 2024 12:16
@anpin
Copy link
Contributor Author

anpin commented Apr 18, 2024

tests seem to be rather flaky both on my machine and in CI. containers sometimes fail on startup.

@anpin anpin force-pushed the feature/net8 branch 2 times, most recently from 2c70a81 to 53e1052 Compare April 18, 2024 15:25
@anpin anpin changed the title [WIP] migrate build system to net8 / fake v6 Migrate build system to net8 / fake v6 Apr 18, 2024
@anpin anpin marked this pull request as ready for review April 18, 2024 16:18
@Aaronontheweb
Copy link
Member

@anpin we definitely have some flaky tests - I'll try to take a look at this. Thank you for your hard work

@anpin
Copy link
Contributor Author

anpin commented Apr 18, 2024

@Aaronontheweb my apologies for not discussing this major change upfront, but I need to build the package from source and didn't want to use some temporary scripts

@anpin
Copy link
Contributor Author

anpin commented Apr 18, 2024

regarding failing tests #362

- previous GetContainerLogsAsync method returning stream was marked obsolete, so I replaced it with the IProgress based version
- Added retry loop in case container fails to start (e.g. port is alraedy in use)
@anpin
Copy link
Contributor Author

anpin commented Apr 19, 2024

Let's see if tests would run consistently in CI now. On my machine they do not fail randomly with last commit.

@anpin
Copy link
Contributor Author

anpin commented Apr 19, 2024

updating release notes just to trigger the build again and see if any error would occur

@anpin
Copy link
Contributor Author

anpin commented Apr 19, 2024

seems to be working. @Aaronontheweb please let me know if anything else is required to merge this

Copy link
Member

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

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

LGTM - nice work. Would love it if we could bring some of this stuff to the main Akka.NET project too (it also uses an aging FAKE system)

@@ -7,6 +7,12 @@
"commands": [
"jb"
]
},
"docfx": {
Copy link
Member

Choose a reason for hiding this comment

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

WE can probably get rid of DocFx altogether (it was just part of the template for this repo) but I appreciate you doing this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will probably make CI faster. I figured it could be executed from some over place where the documentation is built, so I tried to keep it comparability reasons

Copy link
Member

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

Choose a reason for hiding this comment

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

LGTM - much better lol

@@ -62,7 +62,7 @@
]
}],
"xref": [
"../tools/msdn.4.5.2/content/msdn.4.5.2.zip"
"https://learn.microsoft.com/en-us/dotnet/.xrefmap.json"
Copy link
Member

Choose a reason for hiding this comment

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

We should do this in the main Akka.NET repo as well - uses the same system (I would love to get off of the old FAKE there too)

Copy link
Member

Choose a reason for hiding this comment

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

LGTM

@Aaronontheweb Aaronontheweb merged commit 48aa3a8 into akkadotnet:dev Apr 19, 2024
3 checks passed
@Aaronontheweb
Copy link
Member

lol this reminds me, I need to finish getting akkadotnet/akka.net#7063 merged in

@anpin
Copy link
Contributor Author

anpin commented Apr 19, 2024

LGTM - nice work. Would love it if we could bring some of this stuff to the main Akka.NET project too (it also uses an aging FAKE system)

Thanks. I can probable take a look at the main repo too.

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.

2 participants