Skip to content

Conversation

lbialy
Copy link
Collaborator

@lbialy lbialy commented Dec 5, 2023

No description provided.

@pawelprazak pawelprazak added impact/broken Something that is difficult or impossible for some people to use kind/missing We are missing a part of functionality compared to upstream area/codegen Schema to code generator area/core The SDK's core code labels Dec 5, 2023
@pawelprazak pawelprazak added this to the 0.2.0 milestone Dec 5, 2023
@lbialy lbialy force-pushed the read-and-get-resource branch from bd3b265 to 17d2a5f Compare December 7, 2023 08:38

final case class CustomResourceOptions private[internal] (
common: CommonResourceOptions,
provider: Option[ProviderResource],
deleteBeforeReplace: Boolean,
additionalSecretOutputs: List[String],
importId: Option[NonEmptyString] // TODO should this be Id?
importId: Option[ResourceId] // TODO should this be Id?
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: the todo note appears to be fixed now :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
importId: Option[ResourceId] // TODO should this be Id?
importId: Option[ResourceId]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it isn't really, resource options require a rewrite

@@ -81,13 +95,13 @@ trait CustomResourceOptionsFactory:
customTimeouts: String | NotProvided = NotProvided, // CustomTimeouts // TODO
// resourceTransformations: List[ResourceTransformation], // TODO
// aliases: List[Output[Alias]], // TODO
urn: String | NotProvided = NotProvided, // TODO better type
urn: URN | NotProvided = NotProvided, // TODO better type
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: the todo appear to be fixed now

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
urn: URN | NotProvided = NotProvided, // TODO better type
urn: URN | NotProvided = NotProvided,

*
* If you want to understand this regex better head to: https://regex101.com/r/o2QWJ3/1
*
* Now hear me out, this regex is a bit complicated, but it's not that bad. Let's break it down. We have to adhere to this grammar:
Copy link
Contributor

Choose a reason for hiding this comment

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

I like your humor, "not that bad regex" is dark, but still funny ;)

Don't worry, one day we'll write a proper parser :)

@lbialy lbialy merged commit d71fb59 into develop Dec 13, 2023
@lbialy lbialy deleted the read-and-get-resource branch December 13, 2023 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/codegen Schema to code generator area/core The SDK's core code impact/broken Something that is difficult or impossible for some people to use kind/missing We are missing a part of functionality compared to upstream
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants