Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the confusing . in the template tests #8392

Merged
merged 1 commit into from
Jun 30, 2022

Conversation

mattleibow
Copy link
Member

Description of Change

Just making life a bit easier without weird dots making the build look like something went wrong.

@mattleibow mattleibow force-pushed the dev/improve-templates-test branch from dfb013d to 90c5272 Compare June 28, 2022 17:34
@mattleibow mattleibow changed the base branch from main to net6.0 June 28, 2022 17:34
@mattleibow mattleibow added the area-infrastructure CI, Maestro / Coherency, upstream dependencies/versions label Jun 28, 2022
@rmarinho rmarinho requested a review from Eilon June 29, 2022 09:22
@rmarinho
Copy link
Member

I think this was added to actually test something. are we fine removing it @Eilon ?

@Eilon
Copy link
Member

Eilon commented Jun 29, 2022

Hmm I'm not sure what exactly this part of the test is doing (it looks quite different from last time I changed it). But, @rmarinho might be onto something: a long, loooooong time ago I updated the test to use "weird" characters to make sure that when the template was created, that the "weird" characters were correctly substituted in the template. For example, if you do dotnet new maui -o Funny.Dot-Dash_Name the template has to be authored correctly to ensure it gets created as something like Funny_Dot_Dash_Name or something like that. I don't remember the rules, but the key thing is that even with "weird" characters you should get a valid compilable template output.

@Eilon Eilon closed this Jun 29, 2022
@Eilon Eilon reopened this Jun 29, 2022
@Eilon
Copy link
Member

Eilon commented Jun 29, 2022

Oops clicked wrong button somehow. Re-opened.

@Eilon Eilon closed this Jun 29, 2022
@Eilon Eilon reopened this Jun 29, 2022
@Eilon
Copy link
Member

Eilon commented Jun 29, 2022

????????? Apparently Ctrl+Enter closes PRs?? That must be new...

@rmarinho
Copy link
Member

@mattleibow what does this fix btw?

@mattleibow
Copy link
Member Author

Nothing technically, but the build logs are weird to read with a dot in the filename and package names.

@mattleibow
Copy link
Member Author

This is not the templates, the is just the ci part of the tests. This does not change customer code but just our test on ci

@mattleibow mattleibow merged commit 670590b into net6.0 Jun 30, 2022
@mattleibow mattleibow deleted the dev/improve-templates-test branch June 30, 2022 13:35
@Eilon
Copy link
Member

Eilon commented Jun 30, 2022

@mattleibow can you confirm that the template test still tries to create templates that have dots in the name? I think that's what the test was doing, but I'm not familiar with what this change affects.

@mattleibow
Copy link
Member Author

The test does not do that because it never did. I added an extra thing last week and wanted to make it cleaner. This test before was to create a project using different TFMs I I had forgotten to remove the dot from net6.0.

But you raise an important Q, what are some dodgy combos that we should be testing in our test. Like starting with numbers or have weird characters. Is it possible to open an issue on that so I can add a test for those cases?

@Eilon
Copy link
Member

Eilon commented Jun 30, 2022

Looks like we never had dots, but at one point I added dashes and spaces: https://github.com/dotnet/maui/pull/2368/files#diff-f987df2e4d48c04c42bb417e36d0d8b1fa9ddd29f10459c98f16d7b9b10f47c5R112

I think as long as a few oddities are used, that covers just about everything, because there are only a few substitution variables allowed depending on whether it's a type name, filename, etc.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 20, 2023
@samhouts samhouts added the fixed-in-6.0.419 Look for this fix in 6.0.419! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-infrastructure CI, Maestro / Coherency, upstream dependencies/versions fixed-in-6.0.419 Look for this fix in 6.0.419!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants