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

Remove IO macros #1929

Merged
merged 7 commits into from
Dec 4, 2024
Merged

Remove IO macros #1929

merged 7 commits into from
Dec 4, 2024

Conversation

bfops
Copy link
Collaborator

@bfops bfops commented Nov 1, 2024

Description of Changes

Remove the spacetimedb version of common macros like println!, per #1819.

API and ABI breaking changes

Yes. This removes several spacetimedb macros. They should be replaced with spacetimedb::log::info!, spacetimedb::log::error!, or spacetimedb::log::debug!.

Expected complexity level and risk

2

Testing

Existing automated tests have been updated.

@bfops bfops mentioned this pull request Nov 1, 2024
3 tasks
@bfops bfops added the api-break label Nov 1, 2024
@bfops bfops marked this pull request as ready for review November 1, 2024 20:24
@bfops bfops requested a review from cloutiertyler November 1, 2024 20:25
@bfops bfops added release-any To be landed in any release window release-1.0 and removed release-any To be landed in any release window labels Nov 1, 2024
Copy link
Contributor

@mamcx mamcx left a comment

Choose a reason for hiding this comment

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

LGTM

@bfops bfops removed the api-break label Nov 11, 2024
@bfops bfops added abi-break A PR that makes an ABI breaking change breaks library compatibility This change breaks the SpacetimeDB library interface api-break A PR that makes an API breaking change and removed abi-break A PR that makes an ABI breaking change labels Dec 2, 2024
Copy link
Contributor

@cloutiertyler cloutiertyler left a comment

Choose a reason for hiding this comment

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

This looks good to me given the desire to do the println!( grep warning.

Merged via the queue into master with commit a5feed0 Dec 4, 2024
8 checks passed
gefjon added a commit that referenced this pull request Jan 9, 2025
Likely by mistake, #1929 rewrote the `quickstart-chat` client
to use `log::info!` and `log::error!` for output.
In addition to needlessly adding complexity to a quickstart project,
this was broken because that program didn't install a logger,
so it produced no output at all.

This commit reverts those changes, returning to `println`/`eprintln` output
in the quickstart-chat client.
We could use `log::` macros and install a logger,
but then we'd have to explain installing a logger in the quickstart guide.

This commit also makes me a codeowner of the `quickstart-chat` project,
both the module and the client.
I think extra scrutiny on these files is important
because they're always supposed to exactly match the code presented in the tutorials,
which prior to this commit they clearly did not.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-break A PR that makes an API breaking change breaks library compatibility This change breaks the SpacetimeDB library interface release-1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants