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

Managed TestCase Properties implemented #2611

Merged
6 commits merged into from
Nov 19, 2020
Merged

Managed TestCase Properties implemented #2611

6 commits merged into from
Nov 19, 2020

Conversation

Haplois
Copy link
Contributor

@Haplois Haplois commented Nov 2, 2020

@Haplois Haplois force-pushed the fqn branch 3 times, most recently from 593aae8 to ce6ea34 Compare November 2, 2020 09:13
@Haplois Haplois changed the title ManagedType and ManagedMethod support added Managed TestCase Properties implemented Nov 2, 2020
@Haplois Haplois force-pushed the fqn branch 2 times, most recently from 0f8f17d to 51a982b Compare November 2, 2020 09:34
* `ManagedType` and `ManagedMethod` support added to `Microsoft.TestPlatform.ObjectModel.TestCase`
@peterwald
Copy link
Member

In general this looks good to me. A couple of overall comments in addition to the inline ones....

  1. I don't know enough about the TestCase property serialization to know how these changes will interact with forward and backward version mismatches. I think it would be prudent to give this some more thought and testing.
  2. Now that the spec has evolved a bit I wonder if we should consider changing the nomenclature slightly. Since FullyQualifiedNameUtilities is really about removing the reliance on FullyQualifiedName (rather than assisting with it), maybe we should consider renaming it to something along the lines of ManagedQualifiedNameUtilities or ManagedNameUtilities or some such. This may reduce some potential confusion given that these changes really have nothing to do with the existing FullyQualifiedName property.

@shyamnamboodiripad shyamnamboodiripad self-requested a review November 4, 2020 19:42
@Haplois Haplois force-pushed the fqn branch 3 times, most recently from 502bc30 to b50bf89 Compare November 5, 2020 17:59
@Haplois Haplois force-pushed the fqn branch 2 times, most recently from 3528015 to 8bcca4e Compare November 15, 2020 22:38
@Haplois Haplois force-pushed the fqn branch 4 times, most recently from 02fa4b4 to e04f770 Compare November 19, 2020 09:22
@Haplois Haplois marked this pull request as ready for review November 19, 2020 10:38
@Haplois Haplois self-assigned this Nov 19, 2020
@ghost
Copy link

ghost commented Nov 19, 2020

Hello @Haplois!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

eng/Versions.props Show resolved Hide resolved
scripts/build.ps1 Show resolved Hide resolved
@nohwnd
Copy link
Member

nohwnd commented Nov 19, 2020

Looks good :)

@ghost ghost merged commit c872e1f into microsoft:master Nov 19, 2020
@Haplois Haplois deleted the fqn branch November 19, 2020 12:13
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants