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

Composer #656

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Composer #656

wants to merge 12 commits into from

Conversation

kapouik
Copy link

@kapouik kapouik commented Mar 9, 2022

Pull Request (PR) description

By default, when the module install composer, it use https://getcomposer.org/composer.phar
It install the latest preview from nighty build of the dev main repository.

I have set a case that allow to download the right version of composer depending from the channel selected. By default, it use the latest stable.

This Pull Request (PR) fixes the following issues

No issue

Copy link
Member

@smortex smortex left a comment

Choose a reason for hiding this comment

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

Apart from the spacing comment which does not match the rest of voxpupuli code, the rest is under your appreciation 😉

manifests/composer.pp Outdated Show resolved Hide resolved
manifests/composer.pp Outdated Show resolved Hide resolved
manifests/composer.pp Outdated Show resolved Hide resolved
Copy link
Member

@root-expert root-expert left a comment

Choose a reason for hiding this comment

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

Hello @kapouik ,

can we get some basic unit tests for this change?

manifests/composer.pp Outdated Show resolved Hide resolved
manifests/composer.pp Outdated Show resolved Hide resolved
@kapouik kapouik requested a review from root-expert May 1, 2022 16:34
@kapouik
Copy link
Author

kapouik commented May 1, 2022

I have prepare basic test : It only check if composer is installed from default fact.

As other test, I can add check if other channel work if I change it. If you have other idea ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants