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

Display update changelog if requested on self-update #56

Merged
merged 6 commits into from
Aug 18, 2024

Conversation

CompeyDev
Copy link
Contributor

This can potentially be expanded to display changelogs for updates on all tools, rather than just Rokit itself.

Closes #19.

Copy link
Collaborator

@filiptibell filiptibell left a comment

Choose a reason for hiding this comment

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

Nice, this will be a great improvement in UX for updating! I'm not familiar with the libraries used here but they look fine to me.
I am slightly concerned about the diff in Cargo.lock though - it seems like it is pulling in large dependencies such as image, an svg renderer, openssl, and some others that we don't need for basic changelogs in the terminal. We probably need to disable some features in these new deps 🤔

@CompeyDev
Copy link
Contributor Author

I am slightly concerned about the diff in Cargo.lock though

I played around with the features for mdcat - but it doesn't improve the changes in Cargo.lock much. There aren't any features on mdcat to disable HTTP requests either. By itself, mdcat isn't much of a library, but more of a CLI tool, which has a library interface too.

I can probably swap it out for another markdown renderer, but I've not found any other alternatives to have satisfactory render outputs.

@filiptibell
Copy link
Collaborator

It seems like mdcat is mostly a light wrapper around pulldown-cmark-mdcat, and that crate has features for these large dependencies that can be disabled. Maybe we can use that directly instead?
Or if all else fails, we could do some very rudimentary markdown rendering of our own using a simpler parser-only markdown crate. I don't think we're going to have release changelogs with anything beyond basic headers and lists for Rokit anytime soon, at least 😄

@CompeyDev
Copy link
Contributor Author

Maybe we can use that directly instead?

I currently am already disabling its features - but I'll try seeing if using it as a direct rather than peer dependency makes any difference.

@filiptibell
Copy link
Collaborator

I currently am already disabling its features - but I'll try seeing if using it as a direct rather than peer dependency makes any difference.

Yeah, you'll have to remove mdcat since it uses default (all) features for that crate. The cargo book section on feature unification clarifies this a bit - features are always treated as additive and any one of your other dependencies enabling them will mean that they're enabled across the board

* Now directly uses pulldown-cmark-mdcat instead of using mdcat's
  exported wrapper for it
* Updated pulldown-cmark-mdcat to v2.3.0
Copy link
Collaborator

@filiptibell filiptibell left a comment

Choose a reason for hiding this comment

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

LGTM 👍 just need the conflict resolved

@filiptibell filiptibell merged commit d5256ea into rojo-rbx:main Aug 18, 2024
5 checks passed
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.

Show release notes when running self-update command
2 participants