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

- changes with by by for indexers replacements #2564

Merged
merged 1 commit into from
Apr 14, 2023
Merged

- changes with by by for indexers replacements #2564

merged 1 commit into from
Apr 14, 2023

Conversation

baywet
Copy link
Member

@baywet baywet commented Apr 14, 2023

fixes #2528

@baywet baywet self-assigned this Apr 14, 2023
@baywet baywet enabled auto-merge April 14, 2023 17:27
@baywet baywet mentioned this pull request Apr 14, 2023
Closed
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

71.4% 71.4% Coverage
0.0% 0.0% Duplication

MIchaelMainer
MIchaelMainer reviewed Apr 14, 2023
View reviewed changes
Copy link
Member

@MIchaelMainer MIchaelMainer left a comment

Choose a reason for hiding this comment

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

This looks like a breaking change. Is there a version bump missing? Do we need to update release notes with a mention of this change?

@baywet
Copy link
Member Author

baywet commented Apr 14, 2023

@MIchaelMainer thanks for keeping things in check. It's not a breaking change since:

  • Dotnet, the only stable language, uses indexers and doesn't have those compatibility methods (not impacted)
  • other languages are not stable yet and not covered by the support policy.
  • this was already "By" until we decided to change it last week to align with "with" at other places, and revisited that decision yesterday with all PMs (no public release yet).

@baywet baywet merged commit b1b010b into main Apr 14, 2023
@baywet baywet deleted the feature/indexers-by branch April 14, 2023 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MIchaelMainer MIchaelMainer MIchaelMainer approved these changes

@andrueastman andrueastman Awaiting requested review from andrueastman

@darrelmiller darrelmiller Awaiting requested review from darrelmiller

@zengin zengin Awaiting requested review from zengin

@ddyett ddyett Awaiting requested review from ddyett

@peombwa peombwa Awaiting requested review from peombwa

@ramsessanchez ramsessanchez Awaiting requested review from ramsessanchez

@calebkiage calebkiage Awaiting requested review from calebkiage

@Ndiritu Ndiritu Awaiting requested review from Ndiritu

@rkodev rkodev Awaiting requested review from rkodev

@gavinbarron gavinbarron Awaiting requested review from gavinbarron

@coseguera coseguera Awaiting requested review from coseguera

@koros koros Awaiting requested review from koros

Assignees

@baywet baywet

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

All indexers should be in the format client.Repos.ByOwner() instead of client.ReposByOwner()
2 participants
@baywet @MIchaelMainer