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

Fix "neighbor" typo #2130

Merged
merged 2 commits into from Nov 9, 2023
Merged

Fix "neighbor" typo #2130

merged 2 commits into from Nov 9, 2023

Conversation

pmli
Copy link
Member

@pmli pmli commented Jul 25, 2023

  • renames methods with "neighbour"

@pmli pmli added pr:new-feature Introduces a new feature pr:deprecation Deprecates existing functionality labels Jul 25, 2023
@pmli pmli added this to the 2023.2 milestone Jul 25, 2023
@pmli pmli requested a review from sdrave July 25, 2023 19:27
sdrave
sdrave requested changes Aug 14, 2023
View reviewed changes
Copy link
Member

@sdrave sdrave left a comment

Choose a reason for hiding this comment

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

This breaks public API of grids. I don't think eliminating British English is worth the effort. If it is, we should go through a deprecation cycle at least.

@pmli
Copy link
Member Author

pmli commented Aug 15, 2023

I'm not sure if it's worth the effort (that's why I made it a draft PR). If this API is not used so often, maybe it's ok to rename it.

@sdrave
Copy link
Member

sdrave commented Sep 25, 2023

@pmli, how should we proceed? I'm ok with renaming if we properly deprecate the existing method in the upcoming release. I'm also fine with closing the pr and keeping the current situation ..

@pmli pmli marked this pull request as ready for review September 25, 2023 18:54
@pmli
Copy link
Member Author

pmli commented Sep 25, 2023

I would be for deprecating.

@pmli pmli requested a review from sdrave September 25, 2023 18:54
@pmli pmli self-assigned this Nov 2, 2023
@sdrave sdrave added this pull request to the merge queue Nov 9, 2023
Merged via the queue into main with commit 5dfcdc6 Nov 9, 2023
18 checks passed
@sdrave sdrave deleted the neighbor-typo branch November 9, 2023 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sdrave sdrave sdrave approved these changes

Assignees

@pmli pmli

Labels
pr:deprecation Deprecates existing functionality pr:new-feature Introduces a new feature
Projects
None yet
Milestone
2023.2
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@pmli @sdrave