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

Fix .env support when running from phar archive #239

Merged
merged 2 commits into from
Sep 15, 2023

Conversation

stronk7
Copy link
Member

@stronk7 stronk7 commented Sep 11, 2023

To be merged after #238.

With this changes, the moodle-plugin-ci execution
will be able to find a .env file in the same directory that the .phar file containing it.

That way install will be able to create that file and the rest of commands will access to that information via dotenv, avoiding to have to export any env variable manually for phar-based executions.

@stronk7 stronk7 force-pushed the dotenv_support_phar_execution branch from 84d2acf to 1eb5478 Compare September 11, 2023 17:35
@codecov
Copy link

codecov bot commented Sep 11, 2023

Codecov Report

Merging #239 (1bb9c4e) into master (ed8d11b) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master     #239      +/-   ##
============================================
+ Coverage     84.58%   84.62%   +0.04%     
- Complexity      689      694       +5     
============================================
  Files            74       74              
  Lines          2134     2140       +6     
============================================
+ Hits           1805     1811       +6     
  Misses          329      329              
Files Changed Coverage Δ
src/Command/AbstractPluginCommand.php 100.00% <100.00%> (ø)
src/Command/CodeCheckerCommand.php 100.00% <100.00%> (ø)
src/Command/CodeFixerCommand.php 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@stronk7 stronk7 force-pushed the dotenv_support_phar_execution branch 2 times, most recently from cc94e3c to dd24266 Compare September 11, 2023 17:47
We need the 2 CLI scripts and the CodeSniffer.conf file
to be added to the phar, so we can "include" the scripts
for execution and copy the conf file temporarily to
be together with the phar (that's the way CodeSniffer
expects it).

Note this will reduce a little bit code-coverage but in
a future PR I want to add some integration tests to cover
the execution from phar, hardly can be done from unit tests.
With this changes, the moodle-plugin-ci execution
will be able to find a .env file in the same directory
that the .phar file containing it.

That way install will be able to create that file and
the rest of commands will access to that information
via dotenv, avoiding to have to export any env variable
manually for phar-based executions.
kabalin
kabalin previously approved these changes Sep 15, 2023
@stronk7 stronk7 dismissed kabalin’s stale review September 15, 2023 14:23

The merge-base changed after approval.

@stronk7 stronk7 merged commit a9dc987 into moodlehq:master Sep 15, 2023
13 checks passed
@stronk7
Copy link
Member Author

stronk7 commented Sep 15, 2023

Merged, thanks (I just rebased the branch after your review, @kabalin, and that somehow automatically dismissed your review). I've gone ahead.

@stronk7 stronk7 deleted the dotenv_support_phar_execution branch September 15, 2023 15:36
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