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

Add docs section about running in local dev environment #245

Merged
merged 1 commit into from
Oct 2, 2023

Conversation

kabalin
Copy link
Member

@kabalin kabalin commented Sep 29, 2023

No description provided.

@codecov
Copy link

codecov bot commented Sep 29, 2023

Codecov Report

Merging #245 (2932ec0) into master (ace4420) will not change coverage.
Report is 1 commits behind head on master.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #245   +/-   ##
=========================================
  Coverage     84.67%   84.67%           
  Complexity      694      694           
=========================================
  Files            74       74           
  Lines          2147     2147           
=========================================
  Hits           1818     1818           
  Misses          329      329           

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

@stronk7
Copy link
Member

stronk7 commented Sep 30, 2023

Hi Ruslan,

thanks for writing this.

Regarding the PHAR usage, what I do it to put it (a hard-link, I think that there are still some problems with soft links) into all my moodle base directories (note that Moodle's .gitignore has moodle-plugin-ci.phar defined, so it doesn't disturb (maybe I'm used to this because I do the same with composer.phar).

That, also allows me to just update any of the phar files to current one and bump, all them become updated. In any case this is only a detail.

The important point is that, having the phar in the moodle base directory makes things easier and you can run (from base dir):

php moodle-plugin-ci.phar command path/to/plugin

Similarly, the moodle-plugin-ci binary can be added to the $PATH without problems and then it's possible to execute:

moodle-plugin-ci command path/to/plugin

Of course, for both to work, they need to be executed from base directory. Or, alternatively, specify where the base directory is using --moodle /path/to/moodle/base.

So this works:

$ cd enrol (for example)
$ php ../moodle-plugin-ci.phar mustache --moodle=.. ../mod/glossary
$ moodle-plugin-ci mustache --moodle=.. ../mod/glossary

(maybe this is something we could try to improve, making moodle detection better, but that's another story)

Anyway. TL;DR; For your consideration if 1) putting the phar in moodle base dir or 2) adding the binary to the $PATH are good enough to comment about them. Always making clear that, in order for any of them (phar or binary) to work, they must be executed from base dir (dirroot).

Ciao :-)

@stronk7
Copy link
Member

stronk7 commented Sep 30, 2023

Other that that. I like the document. Surely if we start adding more and more details to it (say .env files, say other configurations...) we'll need to move to another docs page, but current proposal fits well in the main page, IMO.

stronk7
stronk7 previously approved these changes Sep 30, 2023
@kabalin
Copy link
Member Author

kabalin commented Sep 30, 2023

Thanks Eloy, I modified the docs slightly to keep moodle-plugin-ci.phar within Moodle dir, thanks for the idea. I think usage example with $PATH and hard links may be suitable if we write a separate doc for that one day, let's keep it simple for index page :)

Copy link
Member

@stronk7 stronk7 left a comment

Choose a reason for hiding this comment

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

Approved, thanks!

@stronk7 stronk7 merged commit 46d30b1 into moodlehq:master Oct 2, 2023
18 checks passed
@kabalin kabalin deleted the local-run branch October 2, 2023 09:41
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