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

feat(showcase): showcase sparkline widget #471

Merged
merged 3 commits into from
Feb 25, 2024

Conversation

mccartney
Copy link
Contributor

Relates to #249.

Adding a showcase entry for Sparkline widget

Copy link
Member

@joshka joshka left a comment

Choose a reason for hiding this comment

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

There's a couple of small issues with this:

  • We use git-lfs to avoid bloating the repo with images, but I think you might have just uploaded the image directly (I'm not super familiar with how to fix this up - possibly @orhun / @kdheepak / @Valentin271 can help?)
  • The sparkline file needs to be formatted with rustfmt to fix up the line lengths

@joshka
Copy link
Member

joshka commented Feb 25, 2024

I'm not sure why, but I was also unable to directly update this branch to fix these issues when I tried.

❯ gpf
To https://github.com/mccartney/ratatui-website.git
 ! [rejected]        showcase-sparkline -> showcase-sparkline (stale info)
error: failed to push some refs to 'https://github.com/mccartney/ratatui-website.git'

@mccartney
Copy link
Contributor Author

I'm not sure why, but I was also unable to directly update this branch to fix these issues when I tried.

That's probably because the PR is from my fork and you don't have permissions there.

@mccartney mccartney requested a review from joshka February 25, 2024 14:47
@kdheepak
Copy link
Contributor

I believe when you make a PR you can choose to allow maintainer to make edits:

image

If you have that checked, I believe we will be able to make changes to this PR even if it from a fork. No need to make a separate PR just for checking that option, but might be good to know for next time :)

@kdheepak
Copy link
Contributor

And I think you have to run the following after installing git lfs:

git lfs install
git lfs pull

and then use git like normal to make commits. I've updated the website contributing section to explicitly state that: https://ratatui.rs/developer-guide/website/

It seems like you've figured this out though, because the test that @Valentin271 set up is passing! So this PR looks good to me.

@joshka joshka merged commit 2a2ec90 into ratatui:main Feb 25, 2024
7 checks passed
@joshka
Copy link
Member

joshka commented Feb 25, 2024

Thanks for fixing this up. I've merged the change.

@mccartney mccartney deleted the showcase-sparkline branch February 26, 2024 05:42
@mccartney
Copy link
Contributor Author

Indeed
https://api.github.com/repos/ratatui-org/website/pulls/471
shows

maintainer_can_modify: false

@mccartney
Copy link
Contributor Author

And TIL it's changeable after the PR is created. At the bottom of the right-hand-side panel:
image

@joshka
Copy link
Member

joshka commented Feb 26, 2024

I could have sworn that I checked that that was ticked on the UI earlier, which was what confused me about this. Heisenbugs...

@mccartney
Copy link
Contributor Author

I could have sworn that I checked that that was ticked on the UI earlier, which was what confused me about this.

And I was convinced I've checked that when creating the PR :)

Anyway. Have a nice week.

@joshka joshka mentioned this pull request Jun 4, 2024
joshka pushed a commit that referenced this pull request Jun 4, 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