Skip to content

Conversation

@zmerp
Copy link
Member

@zmerp zmerp commented Jun 27, 2025

The last refactor before merging broke a couple of things, plus I changed to a different settings format

Tested and working using stable and -dev versions

Comment on lines +7 to +10
pub enum CloseAction {
Close,
CloseWithRequest(ServerRequest),
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I opted for this enum instead of Option to be more clear. Nested Options is never a good idea.

@zmerp zmerp force-pushed the fix-version-popup branch from a055d52 to 7fa6cbc Compare June 27, 2025 15:41
Comment on lines +1551 to 1553
pub struct NewVersionPopupConfig {
pub hide_while_version: String,
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't exactly great instead of just directly putting the String there, but ig it's fine? Idk how else to nicely document that that's the meaning

Copy link
Member Author

@zmerp zmerp Jun 27, 2025

Choose a reason for hiding this comment

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

Yeah that was my thought process. If I used directly string, the parent field would have to convey the meaning, but in this case it was too complex to express. I think as it is now it's more ergonomic for the user.

@zmerp zmerp merged commit be2464e into master Jun 27, 2025
9 checks passed
@zmerp zmerp deleted the fix-version-popup branch June 27, 2025 16:17
zmerp added a commit that referenced this pull request Jun 28, 2025
@zmerp zmerp mentioned this pull request Jun 28, 2025
zmerp added a commit that referenced this pull request Jun 30, 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.

3 participants