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

Trunk LTI 001 - Adding dependencies & LTI Provider folder renaming #8181

Closed
wants to merge 4 commits into from

Conversation

jcopado
Copy link
Contributor

@jcopado jcopado commented Oct 10, 2024

The composer.json file has been modified to include the php-jwt and LTI-PHP dependencies, required in the LTI module to stop including external libraries inside the module itself.

The LTI folder has been renamed to LTIProvider to maintain better consistency in component names.

The composer.json file has been modified to include the php-jwt dependency, required in the LTI module to stop including external libraries inside the module itself.
The composer.json file has been modified to include the LTI-PHP dependency, required in the LTI module to stop including external libraries inside the module itself.
The LTI folder has been renamed to LTIProvider to maintain better consistency in component names.
@jcopado
Copy link
Contributor Author

jcopado commented Oct 10, 2024

Work in progress :)

The composer.lock resulting from the installation of the php-jwt and LTI-PHP dependencies has been added.
@jcopado

This comment was marked as resolved.

@mjansenDatabay mjansenDatabay added improvement dependencies Pull requests that update a dependency file labels Oct 11, 2024
@mjansenDatabay
Copy link
Contributor

Hi @jcopado ,

just one thought on this: If you want to make these dependencies explicit (which is good, thanks a lot) by adding them to the composer.json file, you should label this PR with the "JourFixe" tag and present it on Monday (last JF before the beta release of ILIAS 10.x).

See also: https://github.com/ILIAS-eLearning/ILIAS/blob/trunk/vendor/README.md

Best regards,
Michael

@matthiaskunkel
Copy link
Member

Jour Fixe, 14 OCT 2024: Thanks for the PR. We accept the dependencies and renaming. But we prefer to split up the PR into three PRs, each dedicated to one subject: one for adding 'php-jwt', one for adding 'LTI-PHP' and one for renaming the 'LTI' directory to 'LTI Provider'.
Concerning the two dependencies: please give a short explanation why you need them and add a short information about the status of mainenance of the libraries and which components in ILIAS use them (we assume LTI and LTI Consumer).
The new PRs can be merged to trunk before Coding Completed. No additional discussion in JF needed.

@dsstrassner
Copy link
Contributor

hi @jcopado & @ZallaxDev with the merge of #8232 and #8220 and #8216, this PR could be closed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants