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

Update ENV variable for TRAVIS #4

Merged
merged 3 commits into from
Sep 9, 2022

Conversation

hkorik
Copy link
Contributor

@hkorik hkorik commented Aug 24, 2022

Motivation
With the changes in PR #5 the following error has been detected within a Travis build.

Proposed changes
The current format for retrieving the TRAVIS environment variable doesn't function as expected. Update to use getenv function.

Testing steps

  • Update acquia/blt: v13.5.2.
  • Verify the travis build loads the global TRAVIS environment variable and you don't encounter the error below:
TypeError: Return value of Acquia\BltTravis\Blt\Plugin\EnvironmentDetector\TravisDetector::getCiSettingsFile() must be of the type string, none returned in /home/travis/build/.../vendor/acquia/blt-travis/src/Blt/Plugin/EnvironmentDetector/TravisDetector.php on line 16 #0 [internal function]: Acquia\BltTravis\Blt\Plugin\EnvironmentDetector\TravisDetector::getCiSettingsFile()

Merge requirements

  • Bug, enhancement, or breaking change label applied
  • Manual testing by a reviewer

Sorry, something went wrong.

hkorik added 2 commits August 24, 2022 15:06

Verified

This commit was signed with the committer’s verified signature.
hkorik Hershey Korik

Verified

This commit was signed with the committer’s verified signature.
hkorik Hershey Korik
@bmartinez287
Copy link

This works and we need it as well. +1

@mikemadison13
Copy link
Contributor

part of this pr was already fixed in #5 but I missed the $_ENV change. so, theoretically, the current head of master should already be compatible with the new env. detector. this pr will be ok to help with the environment variable but shouldn't be required to actually fix the breaking change.

@hkorik
Copy link
Contributor Author

hkorik commented Sep 8, 2022

To note this PR was created a week prior to #5. Not sure why this wasn't used in-place of it. Should this be merged in for the $_ENV change?

@mikemadison13
Copy link
Contributor

ah, well I overlooked this then. my apologies. yeah if we could rebase that i can work with @danepowell to get it merged.

Verified

This commit was signed with the committer’s verified signature.
hkorik Hershey Korik
@hkorik hkorik changed the title issue-3-method-type: add method type declaration. Update ENV variable for TRAVIS Sep 8, 2022
@hkorik
Copy link
Contributor Author

hkorik commented Sep 8, 2022

ah, well I overlooked this then. my apologies. yeah if we could rebase that i can work with @danepowell to get it merged.

Latest changes merged in. I also updated the PR details to reflect updated changes.

@danepowell danepowell merged commit 00bf8d7 into acquia:master Sep 9, 2022
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.

None yet

4 participants