fslaborg / flips

Fsharp LInear Programming System
https://flipslibrary.com/#/
MIT License
251 stars 32 forks source link

Namespace / module formatting of tests.fs #138

Closed TysonMN closed 3 years ago

TysonMN commented 3 years ago

What is an advantage of separating the namespace and module as is done in tests.fs? https://github.com/fslaborg/flips/blob/73e4065bd422b69ddf0a10bd55b1e2b897f7fe06/Flips.Tests/Tests.fs#L1-L3

I don't know of any that apply to that file.

The alternative is to combine them like this.

module Flips.Tests.Types

An advantage of combining them is that there is one less level of indentation in the rest of the file.

I am willing to create a PR that combines them and reduce the indentation by one level in the rest of the file.

smoothdeveloper commented 3 years ago

@TysonMN the only one I can think of would be ability to add separate namespaces in the same file.

It doesn't look relevant in this file right now, and if it is bothering you, you could make a PR on the dev branch I guess?

TysonMN commented 3 years ago

Indeed. That is the only advantage that I know of, and as you point out, this is not happening in this file.

I will create the PR. This can always be reversed if there is a future desire for two modules to share the same namespace in this file.

TysonMN commented 3 years ago

The dev branch is giving me problems. There is an obvious syntax error in Flips.Examples, but I can ignore that. The error I would prefer to not ignore is this one for Flips.Solver:

Error FS0078 Unable to find the file
'C:\Code\ThirdParty\flips\bin\netstandard2.0\Flips.dll'
in any of
C:\Users\twilliams\.nuget\packages\netstandard.library\2.0.3\build\netstandard2.0\ref
C:\Code\ThirdParty\flips\Flips.Solver
c:\program files (x86)\microsoft visual studio\2019\enterprise\common7\ide\commonextensions\microsoft\fsharp\

Does anyone know why this is happening?

matthewcrews commented 3 years ago

We have been waiting on updates for Paket and Fake to get full support for .NET 5 which is why the dev branch isn't fully working yet. I'm not sure if that is what is causing those issues though.

TysonMN commented 3 years ago

I am not a huge fan of Paket or Fake.

That aside though, what is the rush to using .NET 5? I recommend keeping the devops stable so that all effort can go into fixing bugs (like #132 that I maybe could have fixed by know but instead have yet to even reproduce) add adding features (like the great redesign coming in version 3).

matthewcrews commented 3 years ago

If you want to fix something in the current version, that would be a PR against main. dev is where we are currently working on the next major version.

I've been busy with some other work but want to come back and formalize how we are managing branches and delivery in a CONTRIBUTION.md file

All thoughts and suggestions are welcome.

And yes, FAKE/Paket have been a bit painful. Not sure I would go with it again. If I knew how to easily remove them, I think I would.

matthewcrews commented 3 years ago

@TysonMN Do you have a recommended branching strategy that you would suggest for development? To date there have not been enough developers to warrant a more formal process. If you have a suggestion, I'd love to hear it!

TysonMN commented 3 years ago

And yes, FAKE/Paket have been a bit painful. Not sure I would go with it again. If I knew how to easily remove them, I think I would.

I could probably do this. In PR https://github.com/hedgehogqa/fsharp-hedgehog/pull/207, I removed Paket for Hedgehog (which is a property testing library that I think is better than FsCheck).

Do you have a recommended branching strategy that you would suggest for development? To date there have not been enough developers to warrant a more formal process.

I don't think a formal process is needed right now either.

Big redesigns are hard though. I am also in the middle of one with Elmish.WPF. Especially because that library is so stable, I recently decided that I would focus exclusively on the branch v4 and not make any changes to master unless someone requested something without a workaround (which is unlikely).

matthewcrews commented 3 years ago

I could probably do this. In PR hedgehogqa/fsharp-hedgehog#207, I removed Paket for Hedgehog (which is a property testing library that I think is better than FsCheck).

If you would like to, please go ahead. I have found that it has made managing dependencies more difficult.

TysonMN commented 3 years ago

Do you have a recommended branching strategy that you would suggest for development? To date there have not been enough developers to warrant a more formal process.

I don't think a formal process is needed right now either.

Big redesigns are hard though. I am also in the middle of one with Elmish.WPF. Especially because that library is so stable, I recently decided that I would focus exclusively on the branch v4 and not make any changes to master unless someone requested something without a workaround (which is unlikely).

At the same time, it would be much easier for me if I could contribute improvements to main. Is that ok if I focus on making improvements to main?

matthewcrews commented 3 years ago

At the same time, it would be much easier for me if I could contribute improvements to main. Is that ok if I focus on making improvements to main?

Yes!

smoothdeveloper commented 3 years ago

@TysonMN, @matthewcrews to share my experience, I'd recommend a workflow where we rebase dev on main once in a while when it doesn't affect any pending work on dev

For any change that doesn't impact the maintenance of current release (which will be mostly bugfixes and maybe small improvements at most), we should preferably make changes in dev, but it doesn't matter so long we frequently rebase dev on main.

In this context, @TysonMN, since there was some issue with building dev out of the box, you can make the adjustment in main.

IMO, we should be careful about fake and paket and they are not blocking, we will end up needing to manage lots of dependencies.

That aside though, what is the rush to using .NET 5? I recommend keeping the devops stable so that all effort can go into fixing bugs

We want the sdk/tooling to be up to date in dev because of what the latest .net brings, we want to use F# 5 compiler, we want to use it in our samples / documentation etc.

I think the situation with net 5, fake and paket is fine now.

TysonMN commented 3 years ago

IMO, we should be careful about fake and paket and they are not blocking, we will end up needing to manage lots of dependencies.

Something is blocking me. Sometimes I can compile the code and other times I cannot. I don't understand what I do differently between the times that work and the times that don't work. My hypothesis is that Paket is to blame, but I don't know this for sure.

My motivation for contributing to Flips will quickly disappear if I can't figure out how to consistently compile the code.

We want the sdk/tooling to be up to date in dev because of what the latest .net brings, we want to use F# 5 compiler, we want to use it in our samples / documentation etc.

Sure, everyone wants the new and shiny. But can you be more specific? What specific feature of the .NET 5 SDK (or maybe some .NET 5 target) provides a meaningfully better experience to developers or users of Flips?

smoothdeveloper commented 3 years ago

#r "nuget: " in sample scripts, F# 5 language features all across the codebase (for example, I believe open type could be part of the things to take in account for certain areas of the API), we want to make sure everything compiles with latest compiler.

The library is small enough to keep close to most recent tooling in dev branch IMHO.

What should be done on the other hand, is fixing the SDK version in the repository through global.json.

TysonMN commented 3 years ago

#r "nuget: " in sample scripts

This is a nice feature. I have not done very much F# scripting. Part of the reason is that I could never figure out how to load a NuGet package. Now it is trivial.

TysonMN commented 3 years ago

This issue isn't very important right now. It is more important to minimize the diff between master and dev.

matthewcrews commented 3 years ago

FYI, the plan is to move all the examples to .fsx files in the future. Eases the delivery.