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

fix build and unit tests #2574

Merged

Conversation

yazeedobaid
Copy link
Collaborator

Description

[WIP] This PR fixes the build by fixing unit tests and dependency resolving.
The unit tests that are fixed related to locating Paket to run Paket.restore command. The issue was about the path where Paket is located. In the fix, I checked only if the path contains paket.exe because this what needs to be ensured to run the restore command.
For the dependency resolving, Suave in build.fsx is pulled from NuGet using the latest version: 2.6 which is built targeting netstandard2.1 which is not compatible with what the project is targeted at, netstandard2.0. Suave has been pinned to 2.5.6 to fix this issue.

Question

I needed to install chocolatey/choco tool in order to get a successful build, should I add Docker files that have all the necessary tools for anyone who uses the repository to get up and running quickly? I needed to install the choco tool only, is there anything else should I consider?

@matthid
Copy link
Member

matthid commented Mar 23, 2021

Thanks a lot!

The Legacy tests still fail, but to be honest maybe it is time to remove them (& the legacy build). It's now almost 3 years since FAKE 5 was released. But I'll leave that up to you.

Regarding the getting started questions: There is some docs on how you to get started with contributing https://fake.build/contributing.html (https://github.com/fsharp/FAKE/blob/release/next/help/markdown/contributing.md) Feel free to update them.

@yazeedobaid
Copy link
Collaborator Author

@matthid I fixed the AppVeyor and Travis builds. For Azure pipeline build, the legacy tests that are failing are tests for the NuGet module, specifically the tests that talk to the NuGet feed. It still uses V1 of the NuGet gallery feed on the legacy and newly migrated version of the NuGet module in FAKE 5. Should I comment out the tests for this module or upgrade the new module and the tests?
The new NuGet module has /// [omit] comment marked on the functions that talk to NuGet, is this comment used in any way in the codebase? Also saw it in other modules.
Also, to your knowledge, is anybody uses the talk to NuGet functionality, and what is the use case could be?

For the getting started guide I'll try to upgrade it after finishing this PR, thanks.

@matthid
Copy link
Member

matthid commented Mar 24, 2021

The omit just means that docs-generation will skip for this member (afaik).

Regarding the tests: IMHO If they impact new modules they probably indicate an issue and should be fixed as some user code will be broken? (moved to new nuget versions?). If they only impact legacy they can go.

@yazeedobaid yazeedobaid force-pushed the fix-build-and-unit-tests branch 2 times, most recently from 1c0d42e to e945f0d Compare March 26, 2021 20:17
@yazeedobaid
Copy link
Collaborator Author

@matthid I have ported the NuGet feed APIs to use V3 of NuGet Gallery API. Also, I have removed the legacy feed tests from the legacy project which were causing the build on Azure to Fail.

@matthid matthid merged commit 493275d into fsprojects:release/next Mar 27, 2021
@matthid
Copy link
Member

matthid commented Mar 27, 2021

Looks good to me, thanks!

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