Skip to content

Conversation

@njlr
Copy link
Contributor

@njlr njlr commented Oct 5, 2025

Extends Cong with FSharp basic types

@Smaug123
Copy link
Contributor

Thanks for this - looks good. The CI for this repo has apparently rotted quite badly, so I'll fix that up first. I'll also drop netframework support and target only netstandard, so we'll be able to get rid of the NETFRAMEWORK clause.

@njlr
Copy link
Contributor Author

njlr commented Oct 10, 2025

Thanks for this - looks good. The CI for this repo has apparently rotted quite badly, so I'll fix that up first. I'll also drop netframework support and target only netstandard, so we'll be able to get rid of the NETFRAMEWORK clause.

I had a go here: #30

@Smaug123
Copy link
Contributor

Thanks - I'm pretty opinionated about CI (sorry) so I'll copy-paste a bunch of that into a fresh branch. In particular I'll add steps ensuring the NuGet pack succeeds, for example, and I'll enable a single step we can use as a gate (as in e.g. https://github.com/G-Research/ApiSurface/blob/ffdd180be34a51fb8294f11750e4bfe775562c9d/.github/workflows/dotnet.yaml#L127-L134).

@njlr
Copy link
Contributor Author

njlr commented Oct 10, 2025

Thanks - I'm pretty opinionated about CI (sorry) so I'll copy-paste a bunch of that into a fresh branch. In particular I'll add steps ensuring the NuGet pack succeeds, for example, and I'll enable a single step we can use as a gate (as in e.g. https://github.com/G-Research/ApiSurface/blob/ffdd180be34a51fb8294f11750e4bfe775562c9d/.github/workflows/dotnet.yaml#L127-L134).

I based it on another GResearch repo FWIW, but no worries 👍

@Smaug123
Copy link
Contributor

We are large, we contain multitudes, I'm afraid. (If it was an F# repo, though, I'd like to remain consistent; which repo was it?)

I'm also now tracking down the configured owner of TypeEquality, because it's definitely misconfigured; I don't have write access right now. There's going to be a delay on this, I'm afraid!

@njlr
Copy link
Contributor Author

njlr commented Oct 10, 2025

We are large, we contain multitudes, I'm afraid. (If it was an F# repo, though, I'd like to remain consistent; which repo was it?)

I'm also now tracking down the configured owner of TypeEquality, because it's definitely misconfigured; I don't have write access right now. There's going to be a delay on this, I'm afraid!

Sorry, I can't recall.

My branch doesn't push any Nuget artefacts, so probably needs to be redone anyways 👍
My objective was to get build and tests working on PRs quickly.

/// returns the type equality for the corresponding Map type.
val map<'k1, 'v1, 'k2, 'v2 when 'k1 : comparison and 'k2 : comparison> : Teq<'k1, 'k2> -> Teq<'v1, 'v2> -> Teq<Map<'k1, 'v1>, Map<'k2, 'v2>>

#if !NETFRAMEWORK
Copy link
Contributor

Choose a reason for hiding this comment

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

Sigh, and all those changes through this FSI file too, I'm afraid!

njlr and others added 7 commits October 11, 2025 09:03
Co-authored-by: Patrick Stevens <3138005+Smaug123@users.noreply.github.com>
Co-authored-by: Patrick Stevens <3138005+Smaug123@users.noreply.github.com>
Co-authored-by: Patrick Stevens <3138005+Smaug123@users.noreply.github.com>
Co-authored-by: Patrick Stevens <3138005+Smaug123@users.noreply.github.com>
Co-authored-by: Patrick Stevens <3138005+Smaug123@users.noreply.github.com>
Co-authored-by: Patrick Stevens <3138005+Smaug123@users.noreply.github.com>
@Smaug123
Copy link
Contributor

Sorry, I merged master into the branch, not realising that the changes were incomplete.

believeMe prf

/// Given a type equality between two seq types, returns the type equality on the corresponding element types.
let seqOf<'a, 'b> (prf : Teq<'a seq, 'b seq>) : Teq<'a, 'b> =
Copy link
Contributor

@Smaug123 Smaug123 Oct 11, 2025

Choose a reason for hiding this comment

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

Actually I think this one is unsound. Teq.seqOf-shaped things in general only work when the types are sealed. This function lets you construct a Teq<'a list, 'a ResizeArray>. The others all look fine to me.

(fst fstPrf)
(snd sndPrf)

/// Given a type equality between two types, returns the type equality on the corresponding Async types.
Copy link
Contributor

Choose a reason for hiding this comment

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

I know you're just being consistent with the rest of this file, but there's already an FSI file, so these doc comments aren't going to be surfaced anywhere. Probably worth just omitting them. (I only say this because I have incoming comments about docstrings, and it's not worth making the same markups twice.)

Co-authored-by: Patrick Stevens <3138005+Smaug123@users.noreply.github.com>
njlr and others added 7 commits October 11, 2025 17:41
Co-authored-by: Patrick Stevens <3138005+Smaug123@users.noreply.github.com>
Co-authored-by: Patrick Stevens <3138005+Smaug123@users.noreply.github.com>
Co-authored-by: Patrick Stevens <3138005+Smaug123@users.noreply.github.com>
Co-authored-by: Patrick Stevens <3138005+Smaug123@users.noreply.github.com>
@Smaug123
Copy link
Contributor

Thanks! Merge at will.

@njlr
Copy link
Contributor Author

njlr commented Oct 11, 2025

Thanks! Merge at will.

Who can merge? 😅

@Smaug123 Smaug123 merged commit 1e1aafe into G-Research:master Oct 11, 2025
7 checks passed
@Smaug123
Copy link
Contributor

Sorry - I thought you would get the button!

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