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.

Sign up for GitHub

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

Jump to bottom

Feat: replace spinner #807

Merged
merged 2 commits into from
Oct 30, 2019
Merged

Feat: replace spinner #807

merged 2 commits into from
Oct 30, 2019

Conversation

lili2311
Copy link
Contributor

@lili2311 lili2311 commented Oct 8, 2019 •
edited
Loading

  • Ready for review
  • Follows CONTRIBUTING rules
  • Reviewed by Snyk internal team

What does this PR do?

  • Introduce external spinner lib gradually

@lili2311 lili2311 requested a review from a team as a code owner October 8, 2019 12:00
@ghost ghost requested review from gitphill and lwywoo October 8, 2019 12:00
@lili2311 lili2311 force-pushed the feat/replace-spinner branch 2 times, most recently from 9a0bba4 to 0ee4576 Compare October 8, 2019 14:59
@lili2311 lili2311 self-assigned this Oct 8, 2019
@lili2311 lili2311 added the 🚧 WIP Work In Progress label Oct 8, 2019
@lili2311
Copy link
Contributor Author

lili2311 commented Oct 9, 2019

Smaller PRs:

@lili2311 lili2311 force-pushed the feat/replace-spinner branch 14 times, most recently from 1c0bd81 to c107b6d Compare October 28, 2019 14:24
@lili2311 lili2311 removed the 🚧 WIP Work In Progress label Oct 28, 2019
@lili2311 lili2311 changed the title Feat/replace spinner Feat: replace spinner Oct 29, 2019
@lili2311 lili2311 mentioned this pull request Oct 29, 2019
Closed
3 tasks
@lili2311
Copy link
Contributor Author

@miiila this is the same as the #807 tested for performance, and CLI test runs are hard to time they are wildly different dependent on time of the day and how busy things are. But from what I had it was maybe on average half a second slower, but user feel was the same.

@lili2311
Copy link
Contributor Author

@miiila ok to merge?

@lili2311 lili2311 merged commit 2248b5c into master Oct 30, 2019
@lili2311 lili2311 deleted the feat/replace-spinner branch October 30, 2019 14:12
@snyksec
Copy link

snyksec commented Oct 30, 2019

🎉 This PR is included in version 1.240.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@lili2311 lili2311 mentioned this pull request Oct 31, 2019
Merged
3 tasks
lili2311
lili2311 commented Oct 31, 2019
View reviewed changes
test/system/remote-package.test.ts
@@ -109,38 +118,6 @@ test('multiple test arguments', async (t) => {
'successfully tested semver@4, qs@1',
);
}

try {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

unnessesary extra tests

@snyk-bot snyk-bot mentioned this pull request Mar 9, 2022
Closed
@snyk-bot snyk-bot mentioned this pull request Aug 31, 2022
Closed
@EitanGayor EitanGayor mentioned this pull request Feb 20, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miiila miiila miiila approved these changes

@gitphill gitphill gitphill approved these changes

@lwywoo lwywoo Awaiting requested review from lwywoo

Assignees

@lili2311 lili2311

Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lili2311 @snyksec @miiila @gitphill