Skip to content

Shorten client:check and service:check output in CI #1404

Merged
jgzuke merged 10 commits intomasterfrom
jgzuke/hide-some-cli-output
Aug 19, 2019
Merged

Shorten client:check and service:check output in CI #1404
jgzuke merged 10 commits intomasterfrom
jgzuke/hide-some-cli-output

Conversation

@jgzuke
Copy link
Copy Markdown
Contributor

@jgzuke jgzuke commented Jul 16, 2019

TODO:

  • Update CHANGELOG.md* with your change (include reference to issue & this PR)
  • Make sure all of the significant new logic is covered by tests
  • Rebase your changes on master so that they can be merged easily
  • Make sure all tests and linter rules pass

*Make sure changelog entries note which project(s) has been affected. See older entries for examples on what this looks like.

@jgzuke jgzuke requested a review from evans July 18, 2019 20:31
@jgzuke
Copy link
Copy Markdown
Contributor Author

jgzuke commented Jul 18, 2019

Waiting for DefinitelyTyped/DefinitelyTyped#36924 for correct types here

@jgzuke
Copy link
Copy Markdown
Contributor Author

jgzuke commented Jul 18, 2019

@evans I put this behind a flag, we could also have this be automatic when the CI env variable is present, im not sure if all test runners respect that flag though

@jgzuke jgzuke force-pushed the jgzuke/hide-some-cli-output branch from 9bd143a to 7ba2314 Compare July 19, 2019 18:19
@jgzuke jgzuke requested a review from JakeDawkins July 22, 2019 19:23
@jgzuke jgzuke force-pushed the jgzuke/hide-some-cli-output branch from c2bbf5b to 9f7322b Compare August 5, 2019 21:14
Copy link
Copy Markdown
Contributor

@evans evans left a comment

Choose a reason for hiding this comment

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

I appreciate the changes! I agree with your comment about automatically setting compact output for ci. The isCI flag from https://github.com/pvdlg/env-ci#readme seems like the ticket! What are you thinking with the implementation, ie this PR or a separate?

Regardless of what we do, dogfooding the compact output within this repository would give me the confidence to approve this PR, knowing that the output works and is useful for ci environments.

Also what's your plan with documentation?

@jgzuke jgzuke force-pushed the jgzuke/hide-some-cli-output branch from 9f7322b to 432a36d Compare August 9, 2019 18:01
@jgzuke jgzuke changed the title Add --compactOutput flag to shorten ci output Shorten client/service:check output in ci Aug 9, 2019
@jgzuke jgzuke changed the title Shorten client/service:check output in ci Shorten client:check and service:check output in CI Aug 9, 2019
@jgzuke jgzuke requested a review from evans August 9, 2019 18:15
@jgzuke
Copy link
Copy Markdown
Contributor Author

jgzuke commented Aug 9, 2019

@jgzuke jgzuke force-pushed the jgzuke/hide-some-cli-output branch from cd7c231 to 0560aa1 Compare August 16, 2019 22:45
@jgzuke jgzuke force-pushed the jgzuke/hide-some-cli-output branch from 3054078 to 0cfceaf Compare August 19, 2019 16:30
Copy link
Copy Markdown
Contributor

@evans evans 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 for putting this together! For a second I was curious how the CompactRender interacts with DEBUG=* apollo service:check and then realized that most users will be running the DEBUG command from their local terminals, so it's a non-issue.

I also recommend thinking about splitting check.test.ts out into two files, one for service and client. Since they depend on a remote server, my hunch is they are the longest tests. Up to you and depends on if they actually are the most time intensive.

Approved, since ^ isn't blocking!


nock.disableNetConnect();

delete process.env.CI;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

re these lines, I think this is a fine solution. Not sure there's a more effective way to simulate this environment

@jgzuke jgzuke merged commit 58266ed into master Aug 19, 2019
@jgzuke jgzuke deleted the jgzuke/hide-some-cli-output branch August 19, 2019 22:02
essaji pushed a commit to essaji/apollo-tooling that referenced this pull request Aug 25, 2019
* Add compact renderer behind flag

* listr types

* type fixes

* type fixes

* update CHANGELOG

* Replace flag with isCi check

* Make client:check compact in CI

* Update CHANGELOG

* Remove extra changes

* Fix CI resetting
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.

2 participants