Skip to content

feat(MessageDb): Allows construction of MessageDb CheckpointStore from NpgsqlDataSource#273

Merged
bartelink merged 8 commits into
jet:masterfrom
njlr:feature/issue-495-message-db-data-source
May 5, 2026
Merged

feat(MessageDb): Allows construction of MessageDb CheckpointStore from NpgsqlDataSource#273
bartelink merged 8 commits into
jet:masterfrom
njlr:feature/issue-495-message-db-data-source

Conversation

@njlr
Copy link
Copy Markdown
Contributor

@njlr njlr commented May 1, 2026

Enables construction of MessageDb components from an NpgsqlDataSource.

Companion PR to jet/equinox#496
Relates to jet/equinox#495

@njlr
Copy link
Copy Markdown
Contributor Author

njlr commented May 1, 2026

@bartelink looks like this package has some semver detection?
Should I revert or bypass?

@bartelink
Copy link
Copy Markdown
Collaborator

Should I revert or bypass?

In general I'd prefer for it to be clean vs dragging around cruft, especially as its highly unlikely that there's a dependency on this (which should have been internal.

Options are to either add an exclusion for that specific warning, or comment out the package backcompat version for the project in question, and then reinstate it but based on 4.1.3 when the next version comes around. While that's more technically correct, I guess adding an exclusion is the way to go as it's really more a bug that an incompativbility?

@njlr
Copy link
Copy Markdown
Contributor Author

njlr commented May 1, 2026

Should I revert or bypass?

In general I'd prefer for it to be clean vs dragging around cruft, especially as its highly unlikely that there's a dependency on this (which should have been internal.

Options are to either add an exclusion for that specific warning, or comment out the package backcompat version for the project in question, and then reinstate it but based on 4.1.3 when the next version comes around. While that's more technically correct, I guess adding an exclusion is the way to go as it's really more a bug that an incompativbility?

Yes this should have been hidden. Perhaps using an fsi file?

@bartelink
Copy link
Copy Markdown
Collaborator

Yes this should have been hidden. Perhaps using an fsi file?

There's been a run at doing that for Equinox in the past - in practice it becomes extremely unwieldy due to specific style choices (keeping lots of stuff in one file so people can easily do a local fork and/or experiment easily, having module Internal pubternal things etc).

So it might have been viable had that been done from the start, but right now it'd be about as likely to yield a pleasing result as adding Fantomas at this point (which has also been tried but given up on in Equinox).

I do agree .fsi files can work very well, e.g. TaskSeq and FSharp.Core do it well, but it's too late for here

Re suppressions, for info this is the technique I was alluding to https://learn.microsoft.com/en-us/dotnet/fundamentals/apicompat/diagnostic-ids#suppression-file

No need to apply it in this instance (unless you are really bored and/or have tokens you wish to burn!)

@njlr
Copy link
Copy Markdown
Contributor Author

njlr commented May 5, 2026

Re suppressions, for info this is the technique I was alluding to https://learn.microsoft.com/en-us/dotnet/fundamentals/apicompat/diagnostic-ids#suppression-file

No need to apply it in this instance (unless you are really bored and/or have tokens you wish to burn!)

It wasn't too difficult! 😅

The build now passes on Linux but not the other platforms due to CosmosStore projects.
These seem unrelated to my change so please advise next steps? 🙏

@bartelink
Copy link
Copy Markdown
Collaborator

Looks like some codegen difference between F#10 and F#6/8 triggered by different SDKs in different environments (could be a compiler bugfix) - IIRC I updted the toolchain in the repo recently.

🤔 it's a matter of balancing having consistent checks independent of build platform and debug/release status as much as possible so people get feedback as early as possible vs the fact that the API compat ultimately only matters on the build from which the artifacts are produced.

@njlr
Copy link
Copy Markdown
Contributor Author

njlr commented May 5, 2026

Looks like some codegen difference between F#10 and F#6/8 triggered by different SDKs in different environments (could be a compiler bugfix) - IIRC I updted the toolchain in the repo recently.

🤔 it's a matter of balancing having consistent checks independent of build platform and debug/release status as much as possible so people get feedback as early as possible vs the fact that the API compat ultimately only matters on the build from which the artifacts are produced.

Luckily I have access to a Mac so was able to "make it green"

@bartelink bartelink merged commit 4987907 into jet:master May 5, 2026
5 checks passed
@bartelink
Copy link
Copy Markdown
Collaborator

Thanks!

@bartelink
Copy link
Copy Markdown
Collaborator

On nuget as 3.0.1 (normally would be 3.1.0 but long fingering FSharp.Core bump on Propulsion package for the moment)

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