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

Allow installing a specific Composer version #321

Merged
merged 2 commits into from
Oct 30, 2020

Conversation

mlocati
Copy link
Contributor

@mlocati mlocati commented Oct 30, 2020


name: ⚙ Improvement
about: What about letting users specify a specific Composer version
labels: enhancement


Description

This PR let users specify a specific Composer version. This is useful in particular to test Composer plugins which should support old Composer versions.

  • I have written test cases for the changes in this pull request
  • I have run npm run format before the commit.
  • I have run npm run lint before the commit.
  • I have run npm run release before the commit.
  • npm test returns with no unit test errors and all code covered.
  • I have checked the edited scripts for syntax.
  • I have tested the changes in an integration test - see https://github.com/mlocati/composer-patcher/actions/runs/337536137

@mlocati
Copy link
Contributor Author

mlocati commented Oct 30, 2020

This script is successful for me:

npm ci
npm run format-check

@mlocati
Copy link
Contributor Author

mlocati commented Oct 30, 2020

This script is successful for me:

Whoops, I forgot to push the changes... see the force push

@mlocati
Copy link
Contributor Author

mlocati commented Oct 30, 2020

It seems that the automatic creation of dist/index.js on my Windows machines mangled the new line characters

@mlocati
Copy link
Contributor Author

mlocati commented Oct 30, 2020

It seems that the automatic creation of dist/index.js on my Windows machines mangled the new line characters

In order to fix that I had to run

git config core.autocrlf true

@mlocati
Copy link
Contributor Author

mlocati commented Oct 30, 2020

About this check:

I have tested the changes in an integration test (If yes, provide workflow link).

How can I test it? What about adding a note about that in https://github.com/shivammathur/setup-php/blob/master/.github/CONTRIBUTING.md ?

@mlocati
Copy link
Contributor Author

mlocati commented Oct 30, 2020

About this check:

I have tested the changes in an integration test (If yes, provide workflow link).

How can I test it? What about adding a note about that in https://github.com/shivammathur/setup-php/blob/master/.github/CONTRIBUTING.md ?

I managed to answer myself:

  1. create a GitHub repository
  2. create a GitHub Action in it
  3. in the YAML file, specify <organization>/<repository>@<branch> instead of shivammathur/setup-php@v2

For example, to test this PR, I used

    steps:
      - name: Setup PHP
        uses: mlocati/setup-php@composer-specific-version
        with:
          php-version: "7.2"
          coverage: none
          tools: composer:v1.7.2

@shivammathur shivammathur merged commit d1510a8 into shivammathur:develop Oct 30, 2020
@mlocati mlocati deleted the composer-specific-version branch October 30, 2020 12:34
@mlocati mlocati restored the composer-specific-version branch October 30, 2020 12:34
@mlocati mlocati deleted the composer-specific-version branch October 30, 2020 12:34
@shivammathur
Copy link
Owner

@mlocati Sorry for a bit of a delay, I have merged this and refactored a bit of it.
It should now work on develop branch, will be in v2 in next release.

- name: Setup PHP
   uses: shivammathur/setup-php@develop
   with:
     php-version: '7.2'
     tools: 'composer:1.7.2'

I will update CONTRIBUTING.md to make it more clear.

@mlocati
Copy link
Contributor Author

mlocati commented Oct 30, 2020

Sorry for a bit of a delay

I wouldn't call that a delay, you are super fast! 😉

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

2 participants