search

Azure / container-apps-deploy-action

GitHub Action for building and deploying Azure Container Apps
MIT License
52 stars 29 forks source link

Convert composite action to typescript #61

Closed snehapar9 closed 1 year ago

snehapar9 commented 1 year ago

This change converts the existing composite action to a typescript based action so we can incorporate the private registry scenario.

daniv-msft commented 1 year ago

I was SURE I made this comment on the PR a couple of weeks back, and I cannot find it so I must have dreamt/forgot to publish it: is it right to assume that most of the code changes are exactly the same as the ADO Task, and that as such we should focus the review on what has changed? If yes, could you please point out the files that are new additions?

snehapar9 commented 1 year ago

I was SURE I made this comment on the PR a couple of weeks back, and I cannot find it so I must have dreamt/forgot to publish it: is it right to assume that most of the code changes are exactly the same as the ADO Task, and that as such we should focus the review on what has changed? If yes, could you please point out the files that are new additions?

From here all the files under src should be the same with some expections AzureAuthenticationHelper.ts is removed in Github Actions because it is not required while using the actions toolkit. GitHubActionsToolHelper.ts has been newly added to this repo. It contains functionality unique to just the Github Actions toolkit, the plan is to create a similar file in the Azure Devops Task to house functionalities unique to the Azure Devops lib.

The dist folder is a new addition but was created by compiling azurecontainerapps.ts and does not need to be reviewed.

daniv-msft commented 1 year ago

I was SURE I made this comment on the PR a couple of weeks back, and I cannot find it so I must have dreamt/forgot to publish it: is it right to assume that most of the code changes are exactly the same as the ADO Task, and that as such we should focus the review on what has changed? If yes, could you please point out the files that are new additions?

From here all the files under src should be the same with some expections AzureAuthenticationHelper.ts is removed in Github Actions because it is not required while using the actions toolkit. GitHubActionsToolHelper.ts has been newly added to this repo. It contains functionality unique to just the Github Actions toolkit, the plan is to create a similar file in the Azure Devops Task to house functionalities unique to the Azure Devops lib.

The dist folder is a new addition but was created by compiling azurecontainerapps.ts and does not need to be reviewed.

Thanks @snehapar9! I'm reviewing specifically GitHubActionsToolHelper.ts in that case.