Skip to content

Conversation

@DavidBoike
Copy link
Member

@DavidBoike DavidBoike commented Aug 14, 2023

Given the PublicClr test is ostensibly to prevent breaking the interface with ServicePulse and ServiceInsight, it seems an actual readout of only the HTTP routes and the methods they map to is a much better indication of that.

Once merged, I intend to rebase #3658 on top of it, as the current diff in the PublicClr API is far too large to get anything useful from.

Copy link
Member

@ramonsmits ramonsmits left a comment

Choose a reason for hiding this comment

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

@DavidBoike is it actually important to know to what the route is mapped to? I see results are ordered 👍🏼 and its all pretty self explanatory

@DavidBoike
Copy link
Member Author

I think it's important to be able to eyeball the controller that maps to that the route that has {id} in it maps to a method that has Guid id or string id in it as a parameter. And if you end up renaming that's fine as that's a simple 1-line diff to deal with.

@DavidBoike DavidBoike merged commit 166cad9 into master Aug 15, 2023
@DavidBoike DavidBoike deleted the api-test branch August 15, 2023 19:12
@danielmarbach
Copy link
Contributor

I was off yesterday and didn't have time to look into this before it got merged. I think this is a good improvement to the previous solution. One thing that I still don't entirely understand, why we are not using something like Pact to verify the contract.

@DavidBoike
Copy link
Member Author

Mostly because something like that would be well outside the scope of our (already overloaded) task force.

@ramonsmits
Copy link
Member

@danielmarbach interesting, I didn't know that project. Do you know if a platform internals observation/opportunity exists for it? I searched for it but not get any results.

@DavidBoike DavidBoike added this to the 5.0.0 milestone Nov 30, 2023
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.

4 participants