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

Don't output console links in CI env #7049

Merged
merged 5 commits into from Dec 4, 2021

Conversation

weirdan
Copy link
Collaborator

@weirdan weirdan commented Dec 3, 2021

They are all but useless there, and are actually causing an issue with GHA output.

Fixes #7047

They are all but useless there, and are actually causing issue with GHA
output.

Fixes vimeo#7047
@weirdan weirdan added the release:fix label Dec 3, 2021
@weirdan weirdan requested a review from orklah Dec 3, 2021
@weirdan weirdan marked this pull request as draft Dec 3, 2021
@weirdan
Copy link
Collaborator Author

weirdan commented Dec 3, 2021

Test failure is legit, but not being able to rely on superglobals makes it quite a bit harder.

weirdan added 3 commits Dec 4, 2021
This has to happen before third-party autoloader has a chance to mess up
global environment.
@weirdan weirdan marked this pull request as ready for review Dec 4, 2021
orklah
orklah approved these changes Dec 4, 2021
@weirdan weirdan merged commit 14dcbc9 into vimeo:master Dec 4, 2021
16 checks passed
@weirdan weirdan deleted the disable-links-in-ci branch Dec 4, 2021
@drupol
Copy link
Contributor

drupol commented Dec 5, 2021

Thanks for fixing the issue !

Question: Could we consider using https://packagist.org/packages/ondram/ci-detector for checking if it is running in CI ?

@orklah
Copy link
Collaborator

orklah commented Dec 5, 2021

Yeah, we could consider it if needed. Do you happen to use an exotic CI environment?

@drupol
Copy link
Contributor

drupol commented Dec 5, 2021

On my side no, I only use Github and Gitlab.

@drupol
Copy link
Contributor

drupol commented Dec 5, 2021

That said, that package support quite a large amount of CI system that I was not even aware of, check their README file !

@orklah
Copy link
Collaborator

orklah commented Dec 5, 2021

That was kinda my point, if we weren't aware they existed, it will be the case for most people too :)

If we have a feedback from a user, we'll consider making the change, but I try not to add dependencies for things I'm not sure are needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants