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.

Sign up for GitHub

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

Jump to bottom

detect MVC versioning in route #1731

Merged
merged 35 commits into from
Jul 22, 2022
Merged

detect MVC versioning in route #1731

merged 35 commits into from
Jul 22, 2022

Conversation

SimonCropp
Copy link
Contributor

@SimonCropp SimonCropp commented Jun 17, 2022 •
edited
Loading

fixes #1637

@SimonCropp SimonCropp marked this pull request as draft June 17, 2022 10:07
@getsentry getsentry deleted a comment from github-actions bot Jun 17, 2022
@bruno-garcia bruno-garcia changed the title detect MVC versioing in route detect MVC versioning in route Jun 17, 2022
lucas-zimerman
lucas-zimerman reviewed Jun 17, 2022
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
SimonCropp and others added 6 commits June 18, 2022 14:30
@SimonCropp
Copy link
Contributor Author

@bruno-garcia @mattjohnsonpint any ideas why this is failing on mac?

@SimonCropp SimonCropp marked this pull request as ready for review June 19, 2022 00:20
bruno-garcia
bruno-garcia approved these changes Jun 19, 2022
View reviewed changes
Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

Not sure why mac CI is timing out, doesn't seem like this PR would affect that

test/Sentry.AspNetCore.Tests/Sentry.AspNetCore.Tests.csproj Outdated Show resolved Hide resolved
@SimonCropp
Copy link
Contributor Author

merged in master to force a new build

@SimonCropp
Copy link
Contributor Author

@bruno-garcia @mattjohnsonpint can one of u try locally on a mac?

@bruno-garcia
Copy link
Member

bruno-garcia commented Jun 26, 2022 •
edited
Loading

yeah we'll need to try locally

image

@mattjohnsonpint mattjohnsonpint self-assigned this Jul 6, 2022
@mattjohnsonpint
Copy link
Contributor

I'll take a look.

mattjohnsonpint
mattjohnsonpint suggested changes Jul 22, 2022
View reviewed changes
Copy link
Contributor

@mattjohnsonpint mattjohnsonpint left a comment

Choose a reason for hiding this comment

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

Want to hold this for the moment. Going to see if I can get it to not hang on mac.

@mattjohnsonpint
Copy link
Contributor

@SimonCropp - I see we already have a lot of other tests that are using the ASP.NET Core TestServer approach (such as here and others). Can you please update these test to use a similar approach? Thanks.

@mattjohnsonpint
Copy link
Contributor

I was able to update it.

@mattjohnsonpint mattjohnsonpint merged commit aad39b9 into main Jul 22, 2022
@mattjohnsonpint mattjohnsonpint deleted the Mvc.Versioning branch July 22, 2022 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bruno-garcia bruno-garcia bruno-garcia approved these changes

@mattjohnsonpint mattjohnsonpint mattjohnsonpint approved these changes

@lucas-zimerman lucas-zimerman Awaiting requested review from lucas-zimerman

Assignees

@SimonCropp SimonCropp

Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@SimonCropp @bruno-garcia @mattjohnsonpint @lucas-zimerman