Apply casing to artifact.xml example #2938
Conversation
|
@guardrex, |
guardrex
changed the title
|
Does this work on the current version @guardrex? /cc @eerhardt @nguerrera |
|
Since comparisons are case insensitive, I think the file will work either way. However, a file produced today will not look like this. Even the spacing that @dasMulli fixed will be messed up. U could sit on this, of course, if they get it into 2.0.1. Otherwise, let me know if u prefer to just have a tracking issue for later, and I convert this into an issue. |
|
So the docs should show the current behavior. I haven't personally tested this yet to see the results. If this is not how the files are generated today, we should log an issue for vnext. |
|
@mairaw The prob I had was that the current output has even bad spacing ... remember? "super embarrassing" lol ... so I corrected the spacing but I left the package names in lowercase. The current example in the topic is a hybrid of reality and @dasMulli fixing it. I split the difference! 😀 To make it look like the actual version, it should look like this (baaaaad spacing) ... <StoreArtifacts>
<Package Id="castle.core" Version ="4.1.0"/>
<Package Id="moq" Version ="4.7.63"/>
<Package Id="newtonsoft.json" Version ="10.0.3"/>
</StoreArtifacts>Problem now tho is that I just tried to repro this locally and hit a big 💥. AFAICT the |
|
So should we just make it clear this is not the auto-generated version but the good version to use? Thanks for investigating this. I can look at this later. |
|
When a dev does a ... dotnet publish --manifest manifest.xmlor via the project file ... <PropertyGroup>
<TargetManifestFiles>manifest.xml</TargetManifestFiles>
</PropertyGroup>In theory (of course! lol), it won't matter and the file will work either way. It just looks bad that the file formatting is all goofy. The more immediate prob tho is that I can't even get the instructions from the topic to work at all. I'm investigating now to see if I can get 2.0.0 CLI to work. |
|
@mairaw 😢 broken. I'll open an issue and see what they say, but it worked with preview 2.0 and it 💥 now. It doesn't error out. It just hangs after ...
|
|
@mairaw kk let's see what they say ... https://github.com/dotnet/cli/issues/7481 |
|
@mairaw I propose both patch updates are closed at #2949 (comment) in favor of a proper set of fixes. If u want to go that way, just say the magic words .... ✨ Make It Happen! ™️ ✨ 😀 |
We have tests that use the normal casing: https://github.com/dotnet/cli/blob/master/TestAssets/TestProjects/FluentProfile/FluentFilterProfile.xml <StoreArtifacts>
<Package Id="Newtonsoft.Json" Version ="9.0.1"/>
</StoreArtifacts> |
|
So if it works, this should be good to merge, right @guardrex? |
|
I think showing the nice looking 2.1 version of the file is best. However if a dev inspects an artifacts.xml today, they'll see the nasty version. Could mark this for 2.1 and wait, but that begs the question if in addition to the lowercase package names should the file shown in the topic also get the foul whitespace .. I mean make it look exactly like the tooling produces it today ... warts and all. 🐸 |
|
@DamianEdwards what do you think? Should we show in the runtime store topic the current behavior or wait until the issue is solved? @guardrex And what it is about the foul whitespace? Does the topic work as-is now? |
|
This is what it looks like until 2.1 ... <StoreArtifacts>
<Package Id="castle.core" Version ="4.1.0"/>
<Package Id="moq" Version ="4.7.63"/>
<Package Id="newtonsoft.json" Version ="10.0.3"/>
</StoreArtifacts>Lower-case package names (which work, of course) and foul spacing around the "Version" attributes (also works). However, it's rather ugly imo and thus kind'a nasty to show to people. I felt it was so ugly that I made a compromise with myself ... I left the lower-case package names but fixed the spacing when I wrote the topic. This PR seeks to fix the lower-case package names. That's really a 2.1 fix ... but ... but ... the foul spacing is really a 2.1 fix, too. The fix is over in dotnet/sdk#1487. |
|
Got it. Thanks for explaining to me @guardrex. So, I think people can get confused when the output doesn't match what they're doing. My vote (even if it's ugly ATM) would be to wait until fix is done. |
|
kk ... I'm just glad it'll be fixed. It's confusing to show package names cased all over the place then not in the file. Might leave a dev wondering, "Do they have to be lower-case for the file to work?" I can patch the current topic and make it the Janet Jackson nasty file layout 😄 if you want ... let me know. Later at 2.1, I can come back for a 🌹 file beautification 🌷 pass. |
|
What do you have against Janet Jackson? 😄 Let's see if @DamianEdwards has a different opinion on this. And I thought the current version already showed the nasty layout. when you wrote, did you beautify it a bit? |
|
Beautified just a bit ... It's the "Ms. Jackson" version right now. I couldn't stand that spacing. It gave me nightmares. 👻 |
@mairaw per dotnet/sdk#1487 🎉 🎈