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(gatsby-cli): move progressbar into ink #14220

Merged
merged 12 commits into from
Jun 18, 2019

Conversation

wardpeet
Copy link
Contributor

Description

Moving progressbar into reporter so it's part of the reporter and not a seperate package. A seperate package like we have now doesn't play well with ink-cli. This will also make the move easier to structured logging and a json formatter.

Related Issues

#11076

@wardpeet wardpeet changed the title Feat/progress bar feat(gatsby-cli): move progressbar into ink May 21, 2019
@wardpeet wardpeet marked this pull request as ready for review May 21, 2019 23:36
@wardpeet wardpeet requested a review from a team as a code owner May 21, 2019 23:36
@DSchau DSchau added the status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response label May 28, 2019
Copy link
Contributor

@m-allanson m-allanson left a comment

Choose a reason for hiding this comment

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

This looks great! What will happen if you use an older gatsby-plugin-sharp or gatsby-source-filesystem with this version of the cli?

I wonder if this PR should be published under tags so we can do some manual testing with CI, different node versions and different plugin versions?

I think the design of the progress bars feels a bit busy with progress bar, item counter, elapsed time and percentage. But I realise it's like that already :) let's not worry about it for now, we can always revisit in a later PR.

@KyleAMathews
Copy link
Contributor

@wardpeet can you post a gif of how this looks now?

@wardpeet
Copy link
Contributor Author

wardpeet commented Jun 5, 2019

old behaviour: (you can see how it's fighting for real estate in the cli)
progressbar - old

new behaviour:
ink-progressbar

@wardpeet
Copy link
Contributor Author

wardpeet commented Jun 5, 2019

it looks like ink has some issues repainting on windows, unsure how it behaves on mac/linux.

@m-allanson
Copy link
Contributor

@ward This looks good! Did you find any more info about the Windows repainting issue?

@wardpeet
Copy link
Contributor Author

I think it's an issue with ink and nothing we can do about. It mostly happens when to activities are running.

@sidharthachatterjee
Copy link
Contributor

@wardpeet Shall we open an issue in ink for this? Is this a blocker or can we merge this in for now?

@wardpeet
Copy link
Contributor Author

it's not a blocker, i'll write an upstream issue. It's still better what we have now.

Copy link
Contributor

@sidharthachatterjee sidharthachatterjee left a comment

Choose a reason for hiding this comment

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

Thank you @wardpeet 🥇

@sidharthachatterjee sidharthachatterjee merged commit 967597c into gatsbyjs:master Jun 18, 2019
@sidharthachatterjee
Copy link
Contributor

Published in gatsby-cli@2.6.8

wardpeet added a commit that referenced this pull request Jun 18, 2019
mxxk pushed a commit to mxxk/gatsby that referenced this pull request Jun 21, 2019
johno pushed a commit to johno/gatsby that referenced this pull request Jul 17, 2019
@wardpeet wardpeet deleted the feat/progress-bar branch September 23, 2020 18:50
mwfrost pushed a commit to mwfrost/gatsby that referenced this pull request Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants