Skip to content

Fixes potential duplicate operation ids of navigation property paths with overloaded composable functions - #597

Merged
Irvine Sunday (irvinesunday) merged 4 commits into
masterfrom
is/composable-fxns-nav-props-fix
Oct 17, 2024
Merged

Fixes potential duplicate operation ids of navigation property paths with overloaded composable functions#597
Irvine Sunday (irvinesunday) merged 4 commits into
masterfrom
is/composable-fxns-nav-props-fix

Conversation

@irvinesunday

@irvinesundayIrvine Sunday (irvinesunday) commented Oct 16, 2024

Copy link
Copy Markdown
Contributor

Fixes#596

andrueastman
Andrew Omondi (andrueastman) reviewed Oct 17, 2024
View reviewed changes

@andrueastmanAndrew Omondi (andrueastman) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a suggestion. Otherwise looks good from my end.

I believe to fix the sonar coverage reporting you will need to update this line to end with coverage.net8.0.opencover.xml as the net7.0 coverage artifact no longer exists...

OpenAPI.NET.OData/.github/workflows/sonarcloud.yml

Line 63 in 78f6629

dotnet tool run dotnet-sonarscanner begin /k:"microsoft_OpenAPI.NET.OData" /o:"microsoft" /d:sonar.token="${{ secrets.SONAR_TOKEN }}" /d:sonar.host.url="https://sonarcloud.io" /d:sonar.cs.opencover.reportsPaths="test/**/coverage.net7.0.opencover.xml"

Comment threadsrc/Microsoft.OpenApi.OData.Reader/Common/EdmModelHelper.cs Outdated
baywet
Vincent Biret (baywet) requested changes Oct 17, 2024
View reviewed changes
Comment threadsrc/Microsoft.OpenApi.OData.Reader/Common/EdmModelHelper.cs Outdated
Comment threadsrc/Microsoft.OpenApi.OData.Reader/Common/EdmModelHelper.cs Outdated
@baywet@andrueastman
chore: adds string comparison
af37f50
Co-authored-by: Andrew Omondi <andrueastman@users.noreply.github.com>
baywet
Vincent Biret (baywet) approved these changes Oct 17, 2024
View reviewed changes

@baywetVincent Biret (baywet) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm a bit surprised we don't have to refresh the integration tests with those changes. But thanks for the quick turnaround on this one.

@baywet

Vincent Biret (baywet) commented Oct 17, 2024

Copy link
Copy Markdown
Member

Andrew Omondi (@andrueastman) thanks for the suggestion, quickly put #598 together

@sonarqubecloud

sonarqubecloudBot commented Oct 17, 2024

Copy link
Copy Markdown

Quality Gate FailedQuality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@irvinesunday
Irvine Sunday (irvinesunday) deleted the is/composable-fxns-nav-props-fix branch October 17, 2024 14:26
Andrew Omondi (andrueastman) added a commit that referenced this pull request Jan 15, 2025
Andrew Omondi@baywet@andrueastman
Fixes potential duplicate operation ids of navigation property paths …
1a29d40
…with overloaded composable functions (#597)
* Fixes generation of navigation property op ids with overloaded composable fxns
* Adds validation unit test
* Updates release note
* chore: adds string comparison
Co-authored-by: Andrew Omondi <andrueastman@users.noreply.github.com>
---------
Co-authored-by: Vincent Biret <vibiret@microsoft.com>
Co-authored-by: Andrew Omondi <andrueastman@users.noreply.github.com>
Andrew Omondi (andrueastman) pushed a commit that referenced this pull request Jan 15, 2025
Andrew Omondi
Fixes potential duplicate operation ids of navigation property paths …
f7611ae
…with overloaded composable functions (#597)
@andrueastmanAndrew Omondi (andrueastman) mentioned this pull request Jan 15, 2025
Merged
Vincent Biret (baywet) added a commit that referenced this pull request Jan 15, 2025
@baywet
Merge pull request#639from microsoft/andrueastman/syncFixes
4db0cbc
fix: port #597
@andrueastmanAndrew Omondi (andrueastman) mentioned this pull request Feb 7, 2025
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@baywetVincent Biret (baywet)baywet approved these changes
@xuzhgSam Xu (xuzhg)Awaiting requested review from xuzhg
@darrelmillerDarrel (darrelmiller)Awaiting requested review from darrelmiller
@danielmbalukaDaniel Mbaluka (danielmbaluka)Awaiting requested review from danielmbaluka
@gavinbarronGavin Barron (gavinbarron)Awaiting requested review from gavinbarron
@millicentachiengMillicent Achieng (millicentachieng)Awaiting requested review from millicentachieng
+1 more reviewer
@andrueastmanAndrew Omondi (andrueastman)andrueastman approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@irvinesundayIrvine Sunday (irvinesunday)

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

remaining duplicate operation Ids for navigation properties on composable functions

3 participants

@irvinesunday@baywet@andrueastman