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 support for tool jolicode/castor #746

Merged
merged 1 commit into from Jun 23, 2023
Merged

Add support for tool jolicode/castor #746

merged 1 commit into from Jun 23, 2023

Conversation

pyrech
Copy link
Contributor

@pyrech pyrech commented Jun 23, 2023

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
52 of 55 checks passed
@pyrech pyrech deleted the support-castor branch June 23, 2023 17:33
@lyrixx
Copy link

lyrixx commented Jun 23, 2023

That's so cool. Thanks

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

@shivammathur
Copy link
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
Copy link

lyrixx commented Jun 23, 2023

Thanks for the feedback. we'll look at it

Side question: how do you debug window platform?

@pyrech
Copy link
Contributor Author

pyrech commented Jun 23, 2023

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
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
Copy link
Contributor Author

pyrech commented Jun 23, 2023

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

Thanks

@lyrixx
Copy link

lyrixx commented Jul 24, 2023

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
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
Copy link
Contributor Author

pyrech commented Jul 29, 2023

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.

None yet

3 participants