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.

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

Python: adds JSON.ARRTRIM command #2457

Merged
merged 2 commits into from
Oct 27, 2024
Merged

Conversation

shohamazon
Copy link
Collaborator

@shohamazon shohamazon commented Oct 15, 2024

Issue link

This Pull Request is linked to issue (URL): #645

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one issue.
  • Commit message has a detailed description of what changed and why.
  • Tests are added or updated.
  • CHANGELOG.md and documentation files are updated.
  • Destination branch is correct - main or release
  • Commits will be squashed upon merging.

@shohamazon shohamazon requested a review from a team as a code owner October 15, 2024 11:35
@shohamazon shohamazon mentioned this pull request Oct 15, 2024
22 tasks
Copy link
Collaborator

@Yury-Fridlyand Yury-Fridlyand left a comment

Choose a reason for hiding this comment

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

Code LGTM
Please fix CI, resolve conflicts and add changelog

Comment on lines 204 to 207
Trims an array at the specified `path` within the JSON document stored at `key` so that it becomes a subarray [start, end], both inclusive.\n
If `start` < 0, it is treated as 0.\n
If `end` >= size (size of the array), it is treated as size-1.\n
If `start` >= size or `start` > `end`, the array is emptied and 0 is returned.\n
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't include \ns

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it is more readable in the IDE
image

no \n:

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

let me know what you thing, removed for now

@Yury-Fridlyand Yury-Fridlyand added the python Python wrapper label Oct 19, 2024
Signed-off-by: Shoham Elias <[email protected]>
Copy link
Collaborator

@BoazBD BoazBD left a comment

Choose a reason for hiding this comment

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

Great work!

@shohamazon shohamazon merged commit c197f1d into release-1.2 Oct 27, 2024
16 checks passed
@shohamazon shohamazon deleted the python/json.arrtrim branch October 27, 2024 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Python wrapper
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants