Skip to content

Add support for tool jolicode/castor#746

Merged
shivammathur merged 1 commit into
shivammathur:developfrom
pyrech:support-castor
Jun 23, 2023
Merged

Add support for tool jolicode/castor#746
shivammathur merged 1 commit into
shivammathur:developfrom
pyrech:support-castor

Conversation

@pyrech

@pyrech pyrech commented Jun 23, 2023

Copy link
Copy Markdown
Contributor

A Pull Request should be associated with a Discussion.

If you're fixing a bug, adding a new feature or improving something please provide the details in discussions,
so that the development can be pointed in the intended direction.

Related discussion: #745

Further notes in Contribution Guidelines
Thank you for your contribution.

Description

This PR add support for the tool jolicode/castor.

A demo repository is provided here: https://github.com/pyrech/castor-setup-php (see this run for a working demo with my fork)

  • 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.

@shivammathur
shivammathur merged commit a683e80 into shivammathur:develop Jun 23, 2023
@pyrech
pyrech deleted the support-castor branch June 23, 2023 17:33
@lyrixx

lyrixx commented Jun 23, 2023

Copy link
Copy Markdown

That's so cool. Thanks

@shivammathur do you know when you will release it? (No pressure, I'm just asking)

@shivammathur

Copy link
Copy Markdown
Owner

@pyrech @lyrixx
I will try to create a release next week.

A couple of issues I noticed,
It does not print the version when running castor -V when using phar builds.
On Windows, it gets stuck when the action tries to run castor -V

@lyrixx

lyrixx commented Jun 23, 2023

Copy link
Copy Markdown

Thanks for the feedback. we'll look at it

Side question: how do you debug window platform?

@pyrech

pyrech commented Jun 23, 2023

Copy link
Copy Markdown
Contributor Author

Looks like the -V flag works when testing on my demo builds (on ubuntu-latest): https://github.com/pyrech/castor-setup-php/pull/1/checks

image

Do you have a reproducer?

Edit: I updated the workflow to run on ubuntu, windows and darwin, and it's all right : https://github.com/pyrech/castor-setup-php/actions/runs/5359907062/jobs/9724101264?pr=2

image

@shivammathur

Copy link
Copy Markdown
Owner

@pyrech @lyrixx
My bad. I did not copy the castor.php file to the directory I was testing in, After adding the file it is working correctly.

@pyrech

pyrech commented Jun 23, 2023

Copy link
Copy Markdown
Contributor Author

Awesome, good to know. Yes, I think we could improve Castor behavior when no initial castor.php have been created yet.

Thanks

@lyrixx

lyrixx commented Jul 24, 2023

Copy link
Copy Markdown

Hello,

For your information, the -V flag has been fixed, even if no castor.php file exist
Thanks for the report

And do you know when you'll be able to release a new version? Thanks 💛

@shivammathur

Copy link
Copy Markdown
Owner

@pyrech @lyrixx
Sorry for the delay in the release. I was really busy with work at my job.

Released in 2.25.5

@pyrech

pyrech commented Jul 29, 2023

Copy link
Copy Markdown
Contributor Author

No worries, that's awesome, thanks for the release 🎉

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.

3 participants