Skip to content

Conversation

jbrooksuk
Copy link
Contributor

I feel like I've done something wrong here as this doesn't seem to work in my test repository, https://github.com/jbrooksuk/ActionServerlessPHP

But it's a start!

@gingerhot
Copy link
Contributor

gingerhot commented Mar 9, 2021

@jbrooksuk Thank you for contribution! I can see there're some problems:

  1. I updated workflow config, you need rebase the code
  2. You comment with a "#", it should be "//" indeed: 68a791d#diff-696942316c4943717efeb63704eb4f92be0cfa63754ea6f6aa62e186680e32cdR3
  3. all the result here is generated by the action workflow, it is not the ground truth for the test. That means you need not to create such a file. If the action works for the new language, there will be such a generated file after action complete. We just check the generated file manually at present
  4. I advise that you don't support dependency installation at present to realize a minimal support for PHP at first step:https://github.com/JamesForks/ActionServerless/blob/php-support/run_funcs.py#L44
  5. You forked the template repo to test, but the template is using released version v1. Your new code not be released yet. You should just check the action code, don't use the template to test.

Thanks again for your contribution!

@jbrooksuk
Copy link
Contributor Author

Thanks @gingerhot! I have made the changes now.

@jbrooksuk
Copy link
Contributor Author

@gingerhot
Copy link
Contributor

Great! Would you please squash the commits as single commit? Then I will merge this pr. Thank you again.

@jbrooksuk
Copy link
Contributor Author

Done!

@gingerhot gingerhot merged commit 91e1ad2 into gitfx:master Mar 9, 2021
@jbrooksuk jbrooksuk deleted the php-support branch March 9, 2021 12:49
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.

2 participants