Skip to content
This repository has been archived by the owner on Dec 9, 2024. It is now read-only.

fix: Log functions in external deployment #456

Merged
merged 2 commits into from
May 18, 2020
Merged

Conversation

tbarlow12
Copy link
Contributor

What did you implement:

In external deployment (always the case for .NET on linux), the deployed functions were not listed in the publishing process. This PR adds that fix.

How did you implement it:

  • Abstracted out logFunctions method
  • Added call to logFunctions after invoking syncTriggers

How can we verify it:

  • Deploy a function app with provider.deployment.external set to true in serverless.yml
  • You should see the functions listed after publishing

Todos:

Note: Run npm run test:ci to run all validation checks on proposed changes

  • Ensure there are no lint errors.
    Validate via npm run lint
    Note: Some reported issues can be automatically fixed by running npm run lint:fix
  • Write tests and confirm existing functionality is not broken.
    Validate via npm test
  • Write documentation
  • Provide verification config / commands / resources
  • Enable "Allow edits from maintainers" for this PR
  • Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

@tbarlow12 tbarlow12 requested a review from wbreza May 18, 2020 17:35
Copy link
Contributor

@wbreza wbreza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - I'd recommend adding a unit test to validate this change.

await this.syncTriggers(functionApp, response.properties);
await this.logFunctions(functionApp);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unit test to validate this change?

@tbarlow12 tbarlow12 merged commit 379ebdb into dev May 18, 2020
@tbarlow12 tbarlow12 deleted the tbarlow12/log-functions branch May 20, 2020 17:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants