-
Notifications
You must be signed in to change notification settings - Fork 131
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Tested all deployment types and overrides. Tested using manifest with buildpacks
(from RC - https://github.com/mauricebrinkmann/aiproject) and worked fine. Three minor comments, but think we can merge without them done.
this.msgSub = this.messages.subscribe(); | ||
|
||
// Buffer messges until each newline character | ||
let b = ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you could have achieved something similar with
share(),
bufferWhen(m => m.endsWith('\n'))),
map(parts => parts.join(''))
log "github.com/sirupsen/logrus" | ||
) | ||
|
||
type v2Info struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a similar V2Info but in the cloud src/jetstream/repository/interfaces/structs.go
Could they us the same version?
} | ||
|
||
// Get the Cloud Foundry Info | ||
func (c *CFPushApp) setEndpointInfo(config *configv3.Config) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be replaced with something like...
cfEndpointSpec, _ := p.GetEndpointTypeSpec("cf")
cnsiInfo, v2Info, err := cfEndpointSpec.Info(theCNSIrecord.APIEndpoint.String(), true)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, will merge once passed gates
Fixes #4181
This PR updates the CF CLI to 6.49.0.