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 pre/post composer commands #67

Merged
merged 2 commits into from
Nov 26, 2023

Conversation

garak
Copy link
Contributor

@garak garak commented Nov 26, 2023

Fix #66

@OskarStark
Copy link
Owner

Er already have REQUIRE_DEV, can't we use this?

@garak
Copy link
Contributor Author

garak commented Nov 26, 2023

But it's not meant for the same purpose

@OskarStark
Copy link
Owner

I agree, will merge and release tomorrow

@OskarStark
Copy link
Owner

OskarStark commented Nov 26, 2023

On a second thought, what about adding an option for pre and post commands as array? So everyone can execute what he wants? Does this makes sense?

@garak
Copy link
Contributor Author

garak commented Nov 26, 2023

You're right, it could be more flexible indeed.
I'll try to propose something

@garak
Copy link
Contributor Author

garak commented Nov 26, 2023

I changed my PR accordingly, but I'm not 100% confident if I did it the right way (sorry, I'm not very skilled in bash)
Let me know

@OskarStark
Copy link
Owner

sorry, I'm not very skilled in bash

Me neither 😄

Lets see if it works :-)

@OskarStark OskarStark changed the title add an option to allow dev dependencies Add support for pre/post composer commands Nov 26, 2023
@OskarStark OskarStark merged commit 0bc2391 into OskarStark:master Nov 26, 2023
@OskarStark
Copy link
Owner

I released 1.10

@dbu
Copy link
Collaborator

dbu commented Nov 27, 2023

this breaks the build with a bash error about "bad substitution":

i had to downgrade to 1.8.0 to get my build to work again. 1.9.0 seems not installable with docker, dunno why.

@OskarStark
Copy link
Owner

Weird as this was released in 1.10

@dbu
Copy link
Collaborator

dbu commented Nov 28, 2023

i was using latest, so it installed 1.10.0 which has this change and failed.

@OskarStark
Copy link
Owner

@garak does it work for you if you provide your commands?

@dbu
Copy link
Collaborator

dbu commented Nov 28, 2023

note that i do not provide any commands.

i guess it is supposed to work in that case too, but maybe something is wrong. not sure if maybe there is some name clash in github actions when one does not explicitly define the variables.

@OskarStark
Copy link
Owner

note that i do not provide any commands.

Yes, that's why I wanted to know if it works with commands as expected, in this case we must provide a default when someone tries to run the action without commands

@OskarStark
Copy link
Owner

Lets revert this for now

OskarStark added a commit that referenced this pull request Nov 29, 2023
OskarStark added a commit that referenced this pull request Nov 29, 2023
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.

allow more composer options?
3 participants