Skip to content

Add JSON validation support for editing container app envelopes#588

Merged
MicroFish91 merged 7 commits intomainfrom
mwf/intellisense
Jan 17, 2024
Merged

Add JSON validation support for editing container app envelopes#588
MicroFish91 merged 7 commits intomainfrom
mwf/intellisense

Conversation

@MicroFish91
Copy link
Copy Markdown
Contributor

@MicroFish91 MicroFish91 commented Jan 12, 2024

Closes #569

@MicroFish91 MicroFish91 requested a review from a team as a code owner January 12, 2024 18:23
@MicroFish91 MicroFish91 marked this pull request as draft January 12, 2024 18:55
@MicroFish91 MicroFish91 marked this pull request as ready for review January 12, 2024 19:02
@MicroFish91 MicroFish91 changed the title Add Intellisense support for editing container app envelopes Add json validation support for editing container app envelopes Jan 12, 2024
@MicroFish91 MicroFish91 changed the title Add json validation support for editing container app envelopes Add JSON validation support for editing container app envelopes Jan 12, 2024
Copy link
Copy Markdown
Member

@nturinski nturinski left a comment

Choose a reason for hiding this comment

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

Nice, love that everything has descriptions now:
image

The name kind of looks like ass now though:
image

I think it should have a user-friendly name, but we use the above convention as the id or something?

Also a side tangent, but do you know if it'd be possible to give the user auto-complete with intellisense for the values? It's pretty tough to know that I have to enter 1Gi for memory, for instance.

Edit: Actually, I see that some of them do provide that (offering an empty array, for example). I'm not super familiar with the schema, but is there a way to offer some default values without it being an enum? Like, (1Gi, 2Gi for example, but it's not an enum of all of the memory capacities.)

@MicroFish91
Copy link
Copy Markdown
Contributor Author

MicroFish91 commented Jan 16, 2024

Yeah I agree, the name looks pretty bad especially due to how long it is. I'm not very clever with naming 😂, do you happen to have any good naming ideas for this, it needs to be a naming convention that maintains uniqueness across all subscription/rg/ca-name combos for lookup. It'd be cool to do this in a way that's independent from the existing resource names and IDs, but I'm not exactly sure how to do that for this.

Edit: Actually, I see that some of them do provide that (offering an empty array, for example). I'm not super familiar with the schema, but is there a way to offer some default values without it being an enum? Like, (1Gi, 2Gi for example, but it's not an enum of all of the memory capacities.)

Hmm, not 100% sure, but I bet there is, do you think it's worth it for us to make the schema more specific than the one exposed by the ARM Schema repo?

@nturinski
Copy link
Copy Markdown
Member

nturinski commented Jan 16, 2024

Yeah I agree, the name looks pretty bad especially due to how long it is. I'm not very clever with naming 😂, do you happen to have any good naming ideas for this, it needs to be a naming convention that maintains uniqueness across all subscription/rg/ca-name combos for lookup. It'd be cool to do this in a way that's independent from the existing resource names and IDs, but I'm not exactly sure how to do that for this.

I believe that the label doesn't have to be unique. The file collision is based on the full path, which should just be the id that we already show.

This is how view properties handles it.

Container app named naturins-test5
image

Two container apps named the same thing:
image

If you hover, you can see the path is different:
image

@MicroFish91 MicroFish91 merged commit 1518af7 into main Jan 17, 2024
@MicroFish91 MicroFish91 deleted the mwf/intellisense branch January 17, 2024 20:29
@MicroFish91
Copy link
Copy Markdown
Contributor Author

Semi-fixed by adding the standard path separators back in.

@microsoft microsoft locked and limited conversation to collaborators Mar 8, 2024
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.

Add JSON validation support for editing container app envelopes

2 participants