Skip to content

refactor!: remove simulator feature#67

Closed
deadbaed wants to merge 10 commits intoratatui:mainfrom
deadbaed:remove_simulator_feature
Closed

refactor!: remove simulator feature#67
deadbaed wants to merge 10 commits intoratatui:mainfrom
deadbaed:remove_simulator_feature

Conversation

@deadbaed
Copy link
Copy Markdown
Contributor

@deadbaed deadbaed commented Jun 16, 2025

misc

  • added workspace for mousefood, examples, and the future
  • build examples in CI
  • editorconfig for toml files

@deadbaed deadbaed marked this pull request as draft June 16, 2025 22:30
@deadbaed deadbaed force-pushed the remove_simulator_feature branch from fe381d6 to b0c2784 Compare June 16, 2025 22:43
@j-g00da
Copy link
Copy Markdown
Member

j-g00da commented Jun 17, 2025

Leave the --lib in CI for now, the docstrings have to be completely rewritten, then we will remove this.

Maybe I will take a look at no_std and merge it without changing docs, as the docs will have to be changed a lot anyway after your changes - I will open a separate issue for that.

I know we will create some temporary mess on main, but I don't mind this as I don't plan releasing mousefood 0.3.0 before ratatui 0.30.0, which will take more than month probably.

WDYT?

Copy link
Copy Markdown
Member

@j-g00da j-g00da left a comment

Choose a reason for hiding this comment

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

Thanks so much for this! I know it's a draft still but I left some notes.

@j-g00da
Copy link
Copy Markdown
Member

j-g00da commented Jun 17, 2025

No-std changes are on main, you can merge these in now.

deadbaed added 3 commits June 17, 2025 21:02
- Move `simulator` example into its own crate
with all its dependencies

- Remove crate feature `simulator`, code implementation
is moved to the `simulator` example crate

- Created workspace to host `mousefood` crate + examples + future stuff
to keep TOML files indented correctly when using multiple text editors
- Install dependencies, and build examples
- Add YAML files to editorconfig
@deadbaed deadbaed force-pushed the remove_simulator_feature branch from b0c2784 to 711a383 Compare June 17, 2025 19:07
@deadbaed
Copy link
Copy Markdown
Contributor Author

I will merge main into my branch, and fix conflicts

I know we will create some temporary mess on main, but I don't mind this as I don't plan releasing mousefood 0.3.0 before ratatui 0.30.0, which will take more than month probably.
WDYT?

Fine by me! If you want we can make a staging branch before merging everything onto main.

I hope there won't be too much mess, my goal is to never break the build on main!

@j-g00da
Copy link
Copy Markdown
Member

j-g00da commented Jun 17, 2025

Main builds, it's just the docs that are outdated.

@deadbaed
Copy link
Copy Markdown
Contributor Author

deadbaed commented Jun 17, 2025

Hum, for some reason GitHub is not showing the fact I merged main into my branch on thie PR https://github.com/deadbaed/mousefood/commits/remove_simulator_feature/

And GitHub action is not running, what is going on? 😅

Edit: TIL GitHub can need some time to process pushes to branches https://github.blog/changelog/2023-09-26-more-details-provided-when-a-pull-request-is-merged-indirectly-or-is-still-processing-updates/

@deadbaed
Copy link
Copy Markdown
Contributor Author

CI is failing, but it's due to a download error. I cannot seem to restart it for the PR, but I re-ran jobs on my fork and everything went well. https://github.com/deadbaed/mousefood/actions/runs/15716651585

Ready for another round of review!

@deadbaed deadbaed marked this pull request as ready for review June 17, 2025 20:05
deadbaed added 2 commits June 17, 2025 22:26
build-examples: make github actions aware of `cargo build` failures by using `xargs`
- ratatui is no longer re-exported from mousefood anymore
- use mousefood::error::Error
Copy link
Copy Markdown
Member

@j-g00da j-g00da left a comment

Choose a reason for hiding this comment

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

I will take a deeper look tomorrow, but I left some comments

Comment on lines +1 to +7
[workspace]
resolver = "3"
members = [
".",
"examples/*",
]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should create a mousefood directory in the repo and move there the whole src and Cargo.toml, then in the root Cargo.toml we should only have workspace setup.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Works for me.
Do you want to do that in another PR?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think let's do it in this PR, it's already mostly about introducing a workspace setup

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay, just pushed it.

@j-g00da
Copy link
Copy Markdown
Member

j-g00da commented Jun 17, 2025

Btw. nit: readme.md -> README.md
But I would remove this readme file completely, and just move it inside /examples/simulator/src/main.rs
(or at least add #![doc = include_str!("../README.md")] inside main.rs).

@deadbaed
Copy link
Copy Markdown
Contributor Author

But I would remove this readme file completely, and just move it inside /examples/simulator/src/main.rs

I'd prefer to keep the readme, GitHub will render it for us when users browse the repo, might get users to clone the repo and run the examples! That's what I do inside repositories when I see they have examples.

@deadbaed deadbaed force-pushed the remove_simulator_feature branch from 82dc613 to 3666037 Compare June 18, 2025 06:47
@deadbaed deadbaed mentioned this pull request Jun 18, 2025
@j-g00da
Copy link
Copy Markdown
Member

j-g00da commented Jun 18, 2025

We should probably put this in docs in main.rs and generate README.md files for all examples using cargo-rdme later (but this is a topic for another PR).

@deadbaed
Copy link
Copy Markdown
Contributor Author

Did not know about this, looks very promising!

Comment on lines +1 to +13
# EditorConfig is awesome: https://editorconfig.org

# top-most EditorConfig file
root = true

[*]
end_of_line = lf
insert_final_newline = true
charset = utf-8

[*.{toml,yml}]
indent_style = space
indent_size = 2
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a topic for separate PR

@j-g00da j-g00da changed the title Move simulator to its own crate, remove crate feature refactor!: remove simulator feature Jun 18, 2025
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.

Simulator feature flag is semver-incompatible

2 participants