Skip to content

Add dummy menu for the web platform #7

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

Merged
merged 2 commits into from
Aug 8, 2024

Conversation

timsueberkrueb
Copy link

  • Tested on all platforms changed
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

@timsueberkrueb timsueberkrueb marked this pull request as ready for review August 3, 2024 09:45
@@ -212,7 +212,7 @@ atomic-waker = "1"
js-sys = "0.3.64"
wasm-bindgen = "0.2"
wasm-bindgen-futures = "0.4"
web-time = "0.2"
web-time = "1"
Copy link

Choose a reason for hiding this comment

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

I believe pinning to the minor value is more accepted than the major

Copy link
Author

@timsueberkrueb timsueberkrueb Aug 5, 2024

Choose a reason for hiding this comment

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

Could you elaborate? web-time = "1" means any version x such that 1.0.0 <= x < 2.0.0.
I guess you want to set the lower bound to the latest minor version, i.e. web-time = "1.1"?

Copy link

Choose a reason for hiding this comment

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

Was just trying to be helpful but I'll leave it up to the maintainers. If you look around think you'll notice its more common to pin to the minor version (1.1 instead of 1) than the major version out of an abundance of caution. Even open source maintainers can make mistakes and ship breaking changes on accident under minor version bumps.

Additionally, I think dependency version bumps unrelated to feature changes could go in separate PRs.

I'm stoked to see work towards floem supporting the web target!

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the feedback! Note that web-time = "1.1" means 1.1 <= x < 2.0.0 and not 1.1 <= x < 1.2. So this is needed when a feature you rely on has been added in a minor version update. web-time = "1" is equivalent to web-time = "1.0".
Nevertheless, I agree that one needs to be careful with specifying only major versions by making sure no feature added in a later minor release has accidentally been used, so always specifying the latest major.minor as a lower bound is probably a good habit.

Copy link

Choose a reason for hiding this comment

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

Haha yep, I just looked at web-time on crates.io because I hadn't seen it before and noticed it was on 1.1 already

@dzhou121 dzhou121 merged commit c8d3b8f into lapce:master Aug 8, 2024
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.

3 participants