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

fix: Combine rollback log statement into one string #290

Merged
merged 1 commit into from
Sep 3, 2019
Merged

fix: Combine rollback log statement into one string #290

merged 1 commit into from
Sep 3, 2019

Conversation

tbarlow12
Copy link
Contributor

@tbarlow12 tbarlow12 commented Aug 29, 2019 •
edited
Loading

Strings were being logged out of order at random for some reason. Seemed cleaner anyway to just combine them into one string and adjusted tests.

@tbarlow12 tbarlow12 closed this Aug 30, 2019
@tbarlow12 tbarlow12 reopened this Aug 30, 2019
wbreza
wbreza approved these changes Aug 30, 2019
View reviewed changes
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

@mydiemho
Copy link
Contributor

Strings were being logged out of order at random for some reason. Seemed cleaner anyway to just combine them into one string and adjusted tests.

we might want to dig deeper as to why log is out of order, I am seeing this in other places too (see 4th line)

Serverless: Finished uploading blob
Serverless: -> Function App not ready. Retrying...
Serverless: -> Function App not ready. Retrying...
Serverless: -> Function package uploaded successfully
Serverless: -> Function App not ready. Retrying...

@tbarlow12
Copy link
Contributor Author

Strings were being logged out of order at random for some reason. Seemed cleaner anyway to just combine them into one string and adjusted tests.

we might want to dig deeper as to why log is out of order, I am seeing this in other places too (see 4th line)

Serverless: Finished uploading blob
Serverless: -> Function App not ready. Retrying...
Serverless: -> Function App not ready. Retrying...
Serverless: -> Function package uploaded successfully
Serverless: -> Function App not ready. Retrying...

That one I can see happening since it's just on a timer. Other functions might finish before. I think in particular, there might be a Promise.all in that section which would allow the two processes to run at the same time

PIC123
PIC123 approved these changes Sep 3, 2019
View reviewed changes
Copy link
Contributor

@PIC123 PIC123 left a comment

Choose a reason for hiding this comment

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

looks good!

@tbarlow12 tbarlow12 merged commit 5806757 into dev Sep 3, 2019
@tbarlow12 tbarlow12 deleted the tabarlow/rollback-log branch September 3, 2019 22:56
tbarlow12 added a commit that referenced this pull request Sep 13, 2019
@tbarlow12
fix: Combine rollback log statement into one string (#290)
4c4fa78 
Strings were being logged out of order at random for some reason. Seemed cleaner anyway to just combine them into one string and adjusted tests.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@PIC123 PIC123 PIC123 approved these changes

@wbreza wbreza wbreza approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tbarlow12 @mydiemho @wbreza @PIC123