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

build.fsx clean up and enhancements #938

Open
MangelMaxime opened this issue Aug 15, 2024 · 2 comments
Open

build.fsx clean up and enhancements #938

MangelMaxime opened this issue Aug 15, 2024 · 2 comments

Comments

@MangelMaxime
Copy link

Hello,

while working on "my PR" I needed to add a new pipeline to simplify testing the output of the API Docs generations using a test project.

While doing so I saw that some variables are not used in the build.fsx:

// Information about the project to be used at NuGet and in AssemblyInfo files
let project = "FSharp.Formatting"

let summary =
    "A package of libraries for building great F# documentation, samples and blogs"

let license = "Apache 2.0 License"

Are you ok if I send a PR to delete dead code?

For my pipeline addition, I need to work with "complex path" and lengthy CLI. Would you be ok if I was to introduce the 2 packages below?

I used these projects extensively for a while now and it simplified a lot my build script experience while making maintenance easier.

I can create a separate PR to showcase how it would look like.

@nojaf
Copy link
Collaborator

nojaf commented Aug 16, 2024

Hi,

I needed to add a new pipeline

I want to push back on that a little. Once we merge, it's on us to maintain, so we need to be sure it's necessary. Is this really something that needs to be part of the repository? Without more context, it feels like this might be more about your preferred workflow than a true necessity.

Are you ok if I send a PR to delete dead code?

Sure, go ahead.

Would you be ok if I was to introduce the 2 packages below?

My initial reaction is no. Adding new dependencies can lead to issues down the line, especially with dotnet. Things tend to break over time, so we should be cautious about introducing anything new unless it's absolutely necessary.

@MangelMaxime
Copy link
Author

My initial reaction is no. Adding new dependencies can lead to issues down the line, especially with dotnet. Things tend to break over time, so we should be cautious about introducing anything new unless it's absolutely necessary.

As you wish, I think that working with string interpolation for CLI is harder to maintain because you need to escape arguments and things like that.

I want to push back on that a little. Once we merge, it's on us to maintain, so we need to be sure it's necessary. Is this really something that needs to be part of the repository? Without more context, it feels like this might be more about your preferred workflow than a true necessity.

The needs will be explained in the signature-like PR because it is related to that enhancement.

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

No branches or pull requests

2 participants