Skip to content

Merge master from upstream into main - #136

Open
NatElkins wants to merge 7 commits into
cagyirey:mainfrom
NatElkins:merge-main
Open

Merge master from upstream into main#136
NatElkins wants to merge 7 commits into
cagyirey:mainfrom
NatElkins:merge-main

Conversation

@NatElkins

Copy link
Copy Markdown

Proposed Changes

This is a branch that merged the master branch from https://github.com/UnoSD/Pulumi.FSharp.Extensions to this repo.

Types of changes

What types of changes does your code introduce to Pulumi.FSharp.Extensions?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • Build and tests pass locally (well, I built it and it works)
  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have added necessary documentation (if appropriate)

@cagyirey

Copy link
Copy Markdown
Owner

Hey @NatElkins, thanks for the PR! I think we might have some overlapping code regarding CustomResourceOptions computation expressions. I'm open to your approach, love how clean the code is. I'd also like to get your opinion on this solution. I want to make sure that we're covering all of the options for both CustomResourceOptions, ComponentResourceOptions.

@NatElkins

Copy link
Copy Markdown
Author

ha, Well I didn't really have anything to do with that code, it's just this PR from the upstream repo. But I will take a look anyway.

@mvsmal you may also have an opinion, as the author of the new CE.

@mvsmal

mvsmal commented Dec 27, 2024

Copy link
Copy Markdown

@NatElkins The only file in this PR looks ok to me, just a little different formatting. However, this is only the CE, in my original PR I had a few more changes to use the CE in the resources CEs. You might want to study them too.

@cagyirey

cagyirey commented Jan 3, 2025

Copy link
Copy Markdown
Owner

I'm leaning toward @mvsmal's implementation because it means fewer top-level custom operations that could conflict with resource argument names. But I'm going to be quite busy with work and moving for at least Q1, so I'm not sure when I'll get around to testing that change.

I also understand the docs for adding your GH packages creds to paket are missing — it's pretty high priority for me to just get the packages on NuGet but Pulumi has their prefix reserved and UnoSD owns the existing packages. When I do get around to it, my packages will probably have to have a different prefix, just FYI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants